chore: follow up to disposables (microsoft#39510)

Author: pavelfeldman

docs/src/api/class-browsercontext.md

@@ -1021,6 +1021,7 @@ API testing helper associated with this context. Requests made with this API wil
 
 ## async method: BrowserContext.route
 * since: v1.8
+- returns: <[Disposable]>
 
 Routing provides the capability to modify network requests that are made by any page in the browser context. Once route
 is enabled, every request matching the url pattern will stall unless it's continued, fulfilled or aborted.

docs/src/api/class-page.md

@@ -3566,6 +3566,7 @@ API testing helper associated with this page. This method returns the same insta
 
 ## async method: Page.route
 * since: v1.8
+- returns: <[Disposable]>
 
 Routing provides the capability to modify network requests that are made by a page.
 

packages/playwright-client/types/types.d.ts

@@ -4147,7 +4147,7 @@ export interface Page {
      * How often a route should be used. By default it will be used every time.
      */
     times?: number;
-  }): Promise<void>;
+  }): Promise<Disposable>;
 
   /**
    * If specified the network requests that are made in the page will be served from the HAR file. Read more about
@@ -9469,7 +9469,7 @@ export interface BrowserContext {
      * How often a route should be used. By default it will be used every time.
      */
     times?: number;
-  }): Promise<void>;
+  }): Promise<Disposable>;
 
   /**
    * If specified the network requests that are made in the context will be served from the HAR file. Read more about

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

@@ -23,7 +23,7 @@ export { Clock } from './clock';
 export { ConsoleMessage } from './consoleMessage';
 export { Coverage } from './coverage';
 export { Dialog } from './dialog';
-export { Disposable } from './disposable';
+export type { Disposable } from './disposable';
 export { Download } from './download';
 export { Electron, ElectronApplication } from './electron';
 export { FrameLocator, Locator } from './locator';

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

@@ -23,7 +23,7 @@ import { evaluationScript } from './clientHelper';
 import { Clock } from './clock';
 import { ConsoleMessage } from './consoleMessage';
 import { Dialog } from './dialog';
-import { Disposable } from './disposable';
+import { DisposableObject, DisposableStub } from './disposable';
 import { TargetClosedError, parseError } from './errors';
 import { Events } from './events';
 import { APIRequestContext } from './fetch';
@@ -352,25 +352,26 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
 
   async addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any) {
     const source = await evaluationScript(this._platform, script, arg);
-    return Disposable.from((await this._channel.addInitScript({ source })).disposable);
+    return DisposableObject.from((await this._channel.addInitScript({ source })).disposable);
   }
 
-  async exposeBinding(name: string, callback: (source: structs.BindingSource, ...args: any[]) => any, options: { handle?: boolean } = {}): Promise<Disposable> {
+  async exposeBinding(name: string, callback: (source: structs.BindingSource, ...args: any[]) => any, options: { handle?: boolean } = {}): Promise<DisposableObject> {
     const result = await this._channel.exposeBinding({ name, needsHandle: options.handle });
     this._bindings.set(name, callback);
-    return Disposable.from(result.disposable);
+    return DisposableObject.from(result.disposable);
   }
 
-  async exposeFunction(name: string, callback: Function): Promise<Disposable> {
+  async exposeFunction(name: string, callback: Function): Promise<DisposableObject> {
     const result = await this._channel.exposeBinding({ name });
     const binding = (source: structs.BindingSource, ...args: any[]) => callback(...args);
     this._bindings.set(name, binding);
-    return Disposable.from(result.disposable);
+    return DisposableObject.from(result.disposable);
   }
 
-  async route(url: URLMatch, handler: network.RouteHandlerCallback, options: { times?: number } = {}): Promise<void> {
+  async route(url: URLMatch, handler: network.RouteHandlerCallback, options: { times?: number } = {}): Promise<DisposableStub> {
     this._routes.unshift(new network.RouteHandler(this._platform, this._options.baseURL, url, handler, options.times));
     await this._updateInterceptionPatterns({ title: 'Route requests' });
+    return new DisposableStub(() => this.unroute(url, handler));
   }
 
   async routeWebSocket(url: URLMatch, handler: network.WebSocketRouteHandlerCallback): Promise<void> {

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

@@ -24,7 +24,7 @@ import { CDPSession } from './cdpSession';
 import { ChannelOwner } from './channelOwner';
 import { createInstrumentation } from './clientInstrumentation';
 import { Dialog } from './dialog';
-import { Disposable } from './disposable';
+import { DisposableObject } from './disposable';
 import { Electron, ElectronApplication } from './electron';
 import { ElementHandle } from './elementHandle';
 import { TargetClosedError, parseError } from './errors';
@@ -272,7 +272,7 @@ export class Connection extends EventEmitter {
         result = new Dialog(parent, type, guid, initializer);
         break;
       case 'Disposable':
-        result = new Disposable(parent, type, guid, initializer);
+        result = new DisposableObject(parent, type, guid, initializer);
         break;
       case 'Electron':
         result = new Electron(parent, type, guid, initializer);

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

@@ -19,8 +19,12 @@ import { isTargetClosedError } from './errors';
 
 import type * as channels from '@protocol/channels';
 
-export class Disposable<T extends channels.DisposableChannel = channels.DisposableChannel> extends ChannelOwner<T> {
-  static from(channel: channels.DisposableChannel): Disposable {
+export interface Disposable {
+  dispose: () => Promise<void>;
+}
+
+export class DisposableObject<T extends channels.DisposableChannel = channels.DisposableChannel> extends ChannelOwner<T> implements Disposable {
+  static from(channel: channels.DisposableChannel): DisposableObject {
     return (channel as any)._object;
   }
 
@@ -38,3 +42,33 @@ export class Disposable<T extends channels.DisposableChannel = channels.Disposab
     }
   }
 }
+
+export class DisposableStub implements Disposable {
+  private _dispose: (() => Promise<void>) | undefined;
+
+  constructor(dispose: () => Promise<void>) {
+    this._dispose = dispose;
+  }
+
+  async [Symbol.asyncDispose]() {
+    await this.dispose();
+  }
+
+  async dispose() {
+    if (!this._dispose)
+      return;
+    try {
+      const dispose = this._dispose;
+      this._dispose = undefined;
+      await dispose();
+    } catch (e) {
+      if (isTargetClosedError(e))
+        return;
+      throw e;
+    }
+  }
+}
+
+export function disposeAll(disposables: Disposable[]) {
+  return Promise.all(disposables.map(d => d.dispose()));
+}

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

@@ -24,6 +24,7 @@
 
 import type { EventEmitter as EventEmitterType } from 'events';
 import type { Platform } from './platform';
+import type { Disposable } from './disposable';
 
 type EventType = string | symbol;
 type Listener = (...args: any[]) => any;
@@ -397,18 +398,14 @@ function wrappedListener(l: Listener): Listener {
   return (l as any).listener;
 }
 
-export type Disposable = {
-  dispose: () => void;
-};
-
 class EventsHelper {
   static addEventListener(
     emitter: EventEmitterType,
     eventName: (string | symbol),
     handler: (...args: any[]) => void): Disposable {
     emitter.on(eventName, handler);
     return {
-      dispose: () => emitter.removeListener(eventName, handler)
+      dispose: async () => { emitter.removeListener(eventName, handler); }
     };
   }
 }

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

@@ -19,7 +19,7 @@ import { Artifact } from './artifact';
 import { ChannelOwner } from './channelOwner';
 import { evaluationScript } from './clientHelper';
 import { Coverage } from './coverage';
-import { Disposable } from './disposable';
+import { DisposableObject, DisposableStub } from './disposable';
 import { Download } from './download';
 import { ElementHandle, determineScreenshotType } from './elementHandle';
 import { TargetClosedError, isTargetClosedError, parseError, serializeError } from './errors';
@@ -336,13 +336,13 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
     const result = await this._channel.exposeBinding({ name });
     const binding = (source: structs.BindingSource, ...args: any[]) => callback(...args);
     this._bindings.set(name, binding);
-    return Disposable.from(result.disposable);
+    return DisposableObject.from(result.disposable);
   }
 
   async exposeBinding(name: string, callback: (source: structs.BindingSource, ...args: any[]) => any, options: { handle?: boolean } = {}) {
     const result = await this._channel.exposeBinding({ name, needsHandle: options.handle });
     this._bindings.set(name, callback);
-    return Disposable.from(result.disposable);
+    return DisposableObject.from(result.disposable);
   }
 
   async setExtraHTTPHeaders(headers: Headers) {
@@ -510,12 +510,13 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
 
   async addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any) {
     const source = await evaluationScript(this._platform, script, arg);
-    return Disposable.from((await this._channel.addInitScript({ source })).disposable);
+    return DisposableObject.from((await this._channel.addInitScript({ source })).disposable);
   }
 
-  async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number } = {}): Promise<void> {
+  async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number } = {}): Promise<DisposableStub> {
     this._routes.unshift(new RouteHandler(this._platform, this._browserContext._options.baseURL, url, handler, options.times));
     await this._updateInterceptionPatterns({ title: 'Route requests' });
+    return new DisposableStub(() => this.unroute(url, handler));
   }
 
   async routeFromHAR(har: string, options: { url?: string | RegExp, notFound?: 'abort' | 'fallback', update?: boolean, updateContent?: 'attach' | 'embed', updateMode?: 'minimal' | 'full'} = {}): Promise<void> {

packages/playwright-core/src/mcp/browser/context.ts

@@ -16,6 +16,7 @@
 
 import path from 'path';
 
+import { disposeAll } from '../../client/disposable';
 import { eventsHelper } from '../../client/eventEmitter';
 import { debug } from '../../utilsBundle';
 import { escapeWithQuotes } from '../../utils/isomorphic/stringUtils';
@@ -29,7 +30,7 @@ import type * as playwright from '../../..';
 import type { FullConfig } from './config';
 import type { SessionLog } from './sessionLog';
 import type { Tracing } from '../../client/tracing';
-import type { Disposable } from '../../client/eventEmitter';
+import type { Disposable } from '../../client/disposable';
 import type { BrowserContext } from '../../client/browserContext';
 import type { ClientInfo } from '../sdk/server';
 
@@ -88,14 +89,12 @@ export class Context {
   }
 
   async dispose() {
-    for (const disposable of this._disposables)
-      disposable.dispose();
-    this._disposables = [];
+    disposeAll(this._disposables);
     for (const tab of this._tabs)
-      tab.dispose();
+      await tab.dispose();
     this._tabs.length = 0;
     this._currentTab = undefined;
-    this.stopVideoRecording();
+    await this.stopVideoRecording();
   }
 
   tabs(): Tab[] {
@@ -235,15 +234,17 @@ export class Context {
 
   private async _setupRequestInterception(context: playwright.BrowserContext) {
     if (this.config.network?.allowedOrigins?.length) {
-      await context.route('**', route => route.abort('blockedbyclient'));
+      this._disposables.push(await context.route('**', route => route.abort('blockedbyclient')));
 
-      for (const origin of this.config.network.allowedOrigins)
-        await context.route(originOrHostGlob(origin), route => route.continue());
+      for (const origin of this.config.network.allowedOrigins) {
+        const glob = originOrHostGlob(origin);
+        this._disposables.push(await context.route(glob, route => route.continue()));
+      }
     }
 
     if (this.config.network?.blockedOrigins?.length) {
       for (const origin of this.config.network.blockedOrigins)
-        await context.route(originOrHostGlob(origin), route => route.abort('blockedbyclient'));
+        this._disposables.push(await context.route(originOrHostGlob(origin), route => route.abort('blockedbyclient')));
     }
   }
 
@@ -263,9 +264,7 @@ export class Context {
       (browserContext as any)._setAllowedDirectories(allRootPaths(this._clientInfo));
     }
     await this._setupRequestInterception(browserContext);
-    for (const page of browserContext.pages())
-      this._onPageCreated(page);
-    this._disposables.push(eventsHelper.addEventListener(browserContext as BrowserContext, 'page', page => this._onPageCreated(page)));
+
     if (this.config.saveTrace) {
       await (browserContext.tracing as Tracing).start({
         name: 'trace-' + Date.now(),
@@ -281,7 +280,12 @@ export class Context {
     }
     const rootPath = firstRootPath(this._clientInfo);
     for (const initScript of this.config.browser.initScript || [])
-      await browserContext.addInitScript({ path: path.resolve(rootPath, initScript) });
+      this._disposables.push(await browserContext.addInitScript({ path: path.resolve(rootPath, initScript) }));
+
+    for (const page of browserContext.pages())
+      this._onPageCreated(page);
+    this._disposables.push(eventsHelper.addEventListener(browserContext as BrowserContext, 'page', page => this._onPageCreated(page)));
+
     return browserContext;
   }