chore(mcp): remove context factory (1) (microsoft#39449)

Author: pavelfeldman

packages/playwright-core/package.json

@@ -26,6 +26,7 @@
     "./lib/cli/program": "./lib/cli/program.js",
     "./lib/mcpBundle": "./lib/mcpBundle.js",
     "./lib/mcp/exports": "./lib/mcp/exports.js",
+    "./lib/mcp/index": "./lib/mcp/index.js",
     "./lib/remote/playwrightServer": "./lib/remote/playwrightServer.js",
     "./lib/server": "./lib/server/index.js",
     "./lib/server/utils/image_tools/stats": "./lib/server/utils/image_tools/stats.js",

packages/playwright-core/src/cli/daemon/daemon.ts

@@ -25,13 +25,14 @@ import { decorateServer } from '../../server/utils/network';
 import { gracefullyProcessExitDoNotHang } from '../../server/utils/processLauncher';
 
 import { BrowserServerBackend } from '../../mcp/browser/browserServerBackend';
+import { browserTools } from '../../mcp/browser/tools';
 import { SocketConnection } from '../client/socketConnection';
 import { commands } from './commands';
 import { parseCommand } from './command';
 
 import type * as mcp from '../../mcp/exports';
-import type { BrowserContextFactory } from '../../mcp/browser/browserContextFactory';
 import type { FullConfig } from '../../mcp/browser/config';
+import type { BrowserContextFactory } from '../../mcp/browser/browserContextFactory';
 import type { SessionConfig } from '../client/registry';
 
 const daemonDebug = debug('pw:daemon');
@@ -75,18 +76,15 @@ export async function startMcpDaemonServer(
     timestamp: Date.now(),
   };
 
-  const { browserContext, close } = await contextFactory.createContext(clientInfo, new AbortController().signal, {});
+  const browserContext = mcpConfig.browser.isolated ? await contextFactory.createContext(clientInfo) : (await contextFactory.contexts(clientInfo))[0];
   if (!noShutdown) {
     browserContext.on('close', () => {
       daemonDebug('browser closed, shutting down daemon');
       shutdown(0);
     });
   }
 
-  const existingContextFactory = {
-    createContext: () => Promise.resolve({ browserContext, close }),
-  };
-  const backend = new BrowserServerBackend(mcpConfig, existingContextFactory, { allTools: true });
+  const backend = new BrowserServerBackend(mcpConfig, browserContext, browserTools);
   await backend.initialize(clientInfo);
 
   await fs.mkdir(path.dirname(socketPath), { recursive: true });
@@ -140,7 +138,7 @@ export async function startMcpDaemonServer(
     server.listen(socketPath, () => {
       daemonDebug(`daemon server listening on ${socketPath}`);
       resolve(async () => {
-        backend.serverClosed();
+        await backend.dispose();
         await new Promise(cb => server.close(cb));
       });
     });

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

@@ -396,3 +396,21 @@ function unwrapListeners(arr: Listener[]): Listener[] {
 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)
+    };
+  }
+}
+
+export const eventsHelper = EventsHelper;

packages/playwright-core/src/mcp/DEPS.list

@@ -2,6 +2,7 @@
 ../../
 ./sdk/
 ./browser/
+./browser/tools/
 ./extension/
 ../cli/
 ../utilsBundle.ts

packages/playwright-core/src/mcp/browser/DEPS.list

@@ -3,6 +3,7 @@
 ./tools/
 ../sdk/
 ../log.ts
+../../utils/
 ../../utils/isomorphic/
 ../../utilsBundle.ts
 ../../mcpBundle.ts

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

@@ -22,7 +22,7 @@ import path from 'path';
 import * as playwright from '../../..';
 import { registryDirectory } from '../../server/registry/index';
 import { startTraceViewerServer } from '../../server';
-import { logUnhandledError, testDebug } from '../log';
+import { testDebug } from '../log';
 import { outputDir, outputFile } from './config';
 import { firstRootPath } from '../sdk/server';
 
@@ -31,8 +31,6 @@ import type { LaunchOptions, BrowserContextOptions } from '../../client/types';
 import type { ClientInfo } from '../sdk/server';
 
 export function contextFactory(config: FullConfig): BrowserContextFactory {
-  if (config.sharedBrowserContext)
-    return SharedContextFactory.create(config);
   if (config.browser.remoteEndpoint)
     return new RemoteContextFactory(config);
   if (config.browser.cdpEndpoint)
@@ -42,26 +40,19 @@ export function contextFactory(config: FullConfig): BrowserContextFactory {
   return new PersistentContextFactory(config);
 }
 
-export type BrowserContextFactoryResult = {
-  browserContext: playwright.BrowserContext;
-  close: () => Promise<void>;
-};
-
-type CreateContextOptions = {
-  toolName?: string;
-};
-
 export interface BrowserContextFactory {
-  createContext(clientInfo: ClientInfo, abortSignal: AbortSignal, options: CreateContextOptions): Promise<BrowserContextFactoryResult>;
+  contexts(clientInfo: ClientInfo): Promise<playwright.BrowserContext[]>;
+  createContext(clientInfo: ClientInfo): Promise<playwright.BrowserContext>;
 }
 
 export function identityBrowserContextFactory(browserContext: playwright.BrowserContext): BrowserContextFactory {
   return {
-    createContext: async (clientInfo: ClientInfo, abortSignal: AbortSignal, options: CreateContextOptions) => {
-      return {
-        browserContext,
-        close: async () => {}
-      };
+    contexts: async (clientInfo: ClientInfo) => {
+      return [browserContext];
+    },
+
+    createContext: async (clientInfo: ClientInfo) => {
+      return browserContext;
     }
   };
 }
@@ -76,11 +67,11 @@ class BaseContextFactory implements BrowserContextFactory {
     this.config = config;
   }
 
-  protected async _obtainBrowser(clientInfo: ClientInfo, options: CreateContextOptions): Promise<playwright.Browser> {
+  protected async _obtainBrowser(clientInfo: ClientInfo): Promise<playwright.Browser> {
     if (this._browserPromise)
       return this._browserPromise;
     testDebug(`obtain browser (${this._logName})`);
-    this._browserPromise = this._doObtainBrowser(clientInfo, options);
+    this._browserPromise = this._doObtainBrowser(clientInfo);
     void this._browserPromise.then(browser => {
       browser.on('disconnected', () => {
         this._browserPromise = undefined;
@@ -91,43 +82,32 @@ class BaseContextFactory implements BrowserContextFactory {
     return this._browserPromise;
   }
 
-  protected async _doObtainBrowser(clientInfo: ClientInfo, options: CreateContextOptions): Promise<playwright.Browser> {
+  protected async _doObtainBrowser(clientInfo: ClientInfo): Promise<playwright.Browser> {
     throw new Error('Not implemented');
   }
 
-  async createContext(clientInfo: ClientInfo, _: AbortSignal, options: CreateContextOptions): Promise<BrowserContextFactoryResult> {
+  async contexts(clientInfo: ClientInfo): Promise<playwright.BrowserContext[]> {
+    const browser = await this._obtainBrowser(clientInfo);
+    return browser.contexts();
+  }
+
+  async createContext(clientInfo: ClientInfo): Promise<playwright.BrowserContext> {
     testDebug(`create browser context (${this._logName})`);
-    const browser = await this._obtainBrowser(clientInfo, options);
-    const browserContext = await this._doCreateContext(browser, clientInfo);
-    await addInitScript(browserContext, this.config.browser.initScript);
-    return {
-      browserContext,
-      close: () => this._closeBrowserContext(browserContext, browser)
-    };
+    const browser = await this._obtainBrowser(clientInfo);
+    return await this._doCreateContext(browser, clientInfo);
   }
 
   protected async _doCreateContext(browser: playwright.Browser, clientInfo: ClientInfo): Promise<playwright.BrowserContext> {
     throw new Error('Not implemented');
   }
-
-  private async _closeBrowserContext(browserContext: playwright.BrowserContext, browser: playwright.Browser) {
-    testDebug(`close browser context (${this._logName})`);
-    if (browser.contexts().length === 1)
-      this._browserPromise = undefined;
-    await browserContext.close().catch(logUnhandledError);
-    if (browser.contexts().length === 0) {
-      testDebug(`close browser (${this._logName})`);
-      await browser.close().catch(logUnhandledError);
-    }
-  }
 }
 
 class IsolatedContextFactory extends BaseContextFactory {
   constructor(config: FullConfig) {
     super('isolated', config);
   }
 
-  protected override async _doObtainBrowser(clientInfo: ClientInfo, options: CreateContextOptions): Promise<playwright.Browser> {
+  protected override async _doObtainBrowser(clientInfo: ClientInfo): Promise<playwright.Browser> {
     await injectCdpPort(this.config.browser);
     const browserType = playwright[this.config.browser.browserName];
     const tracesDir = await computeTracesDir(this.config, clientInfo);
@@ -185,16 +165,15 @@ class RemoteContextFactory extends BaseContextFactory {
   }
 }
 
-class PersistentContextFactory implements BrowserContextFactory {
-  readonly config: FullConfig;
+class PersistentContextFactory extends BaseContextFactory {
   readonly name = 'persistent';
   readonly description = 'Create a new persistent browser context';
 
   constructor(config: FullConfig) {
-    this.config = config;
+    super('persistent', config);
   }
 
-  async createContext(clientInfo: ClientInfo, abortSignal: AbortSignal, options: CreateContextOptions): Promise<BrowserContextFactoryResult> {
+  protected override async _doObtainBrowser(clientInfo: ClientInfo): Promise<playwright.Browser> {
     await injectCdpPort(this.config.browser);
     testDebug('create browser context (persistent)');
     const userDataDir = this.config.browser.userDataDir ?? await this._createUserDataDir(clientInfo);
@@ -205,8 +184,6 @@ class PersistentContextFactory implements BrowserContextFactory {
     if (await isProfileLocked5Times(userDataDir))
       throw new Error(`Browser is already in use for ${userDataDir}, use --isolated to run multiple instances of the same browser`);
 
-    testDebug('lock user data dir', userDataDir);
-
     const browserType = playwright[this.config.browser.browserName];
     const launchOptions: LaunchOptions & BrowserContextOptions = {
       tracesDir,
@@ -221,9 +198,7 @@ class PersistentContextFactory implements BrowserContextFactory {
     };
     try {
       const browserContext = await browserType.launchPersistentContext(userDataDir, launchOptions);
-      await addInitScript(browserContext, this.config.browser.initScript);
-      const close = () => this._closeBrowserContext(browserContext, userDataDir);
-      return { browserContext, close };
+      return browserContext.browser()!;
     } catch (error: any) {
       if (error.message.includes('Executable doesn\'t exist'))
         throwBrowserIsNotInstalledError(this.config);
@@ -237,15 +212,6 @@ class PersistentContextFactory implements BrowserContextFactory {
     }
   }
 
-  private async _closeBrowserContext(browserContext: playwright.BrowserContext, userDataDir: string) {
-    testDebug('close browser context (persistent)');
-    testDebug('release user data dir', userDataDir);
-    await browserContext.close().catch(() => {});
-    if (process.env.PWMCP_PROFILES_DIR_FOR_TEST && userDataDir.startsWith(process.env.PWMCP_PROFILES_DIR_FOR_TEST))
-      await fs.promises.rm(userDataDir, { recursive: true }).catch(logUnhandledError);
-    testDebug('close browser context complete (persistent)');
-  }
-
   private async _createUserDataDir(clientInfo: ClientInfo) {
     const dir = process.env.PWMCP_PROFILES_DIR_FOR_TEST ?? registryDirectory;
     const browserToken = this.config.browser.launchOptions?.channel ?? this.config.browser?.browserName;
@@ -289,59 +255,6 @@ function createHash(data: string): string {
   return crypto.createHash('sha256').update(data).digest('hex').slice(0, 7);
 }
 
-async function addInitScript(browserContext: playwright.BrowserContext, initScript: string[] | undefined) {
-  for (const scriptPath of initScript ?? [])
-    await browserContext.addInitScript({ path: path.resolve(scriptPath) });
-}
-
-export class SharedContextFactory implements BrowserContextFactory {
-  private _contextPromise: Promise<BrowserContextFactoryResult> | undefined;
-  private _baseFactory: BrowserContextFactory;
-  private static _instance: SharedContextFactory | undefined;
-
-  static create(config: FullConfig) {
-    if (SharedContextFactory._instance)
-      throw new Error('SharedContextFactory already exists');
-    const baseConfig = { ...config, sharedBrowserContext: false };
-    const baseFactory = contextFactory(baseConfig);
-    SharedContextFactory._instance = new SharedContextFactory(baseFactory);
-    return SharedContextFactory._instance;
-  }
-
-  private constructor(baseFactory: BrowserContextFactory) {
-    this._baseFactory = baseFactory;
-  }
-
-  async createContext(clientInfo: ClientInfo, abortSignal: AbortSignal, options: { toolName?: string }): Promise<{ browserContext: playwright.BrowserContext, close: () => Promise<void> }> {
-    if (!this._contextPromise) {
-      testDebug('create shared browser context');
-      this._contextPromise = this._baseFactory.createContext(clientInfo, abortSignal, options);
-    }
-
-    const { browserContext } = await this._contextPromise;
-    testDebug(`shared context client connected`);
-    return {
-      browserContext,
-      close: async () => {
-        testDebug(`shared context client disconnected`);
-      },
-    };
-  }
-
-  static async dispose() {
-    await SharedContextFactory._instance?._dispose();
-  }
-
-  private async _dispose() {
-    const contextPromise = this._contextPromise;
-    this._contextPromise = undefined;
-    if (!contextPromise)
-      return;
-    const { close } = await contextPromise;
-    await close();
-  }
-}
-
 async function computeTracesDir(config: FullConfig, clientInfo: ClientInfo): Promise<string | undefined> {
   return path.resolve(outputDir(config, clientInfo), 'traces');
 }

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

@@ -16,14 +16,13 @@
 
 import { FullConfig } from './config';
 import { Context } from './context';
-import { logUnhandledError } from '../log';
 import { Response } from './response';
 import { SessionLog } from './sessionLog';
-import { browserTools, filteredTools } from './tools';
 import { toMcpTool } from '../sdk/tool';
+import { logUnhandledError } from '../log';
 
+import type * as playwright from '../../..';
 import type { Tool } from './tools/tool';
-import type { BrowserContextFactory } from './browserContextFactory';
 import type * as mcpServer from '../sdk/server';
 import type { ServerBackend } from '../sdk/server';
 
@@ -32,24 +31,27 @@ export class BrowserServerBackend implements ServerBackend {
   private _context: Context | undefined;
   private _sessionLog: SessionLog | undefined;
   private _config: FullConfig;
-  private _browserContextFactory: BrowserContextFactory;
+  readonly browserContext: playwright.BrowserContext;
 
-  constructor(config: FullConfig, factory: BrowserContextFactory, options: { allTools?: boolean, structuredOutput?: boolean } = {}) {
+  constructor(config: FullConfig, browserContext: playwright.BrowserContext, tools: Tool[]) {
     this._config = config;
-    this._browserContextFactory = factory;
-    this._tools = options.allTools ? browserTools : filteredTools(config);
+    this._tools = tools;
+    this.browserContext = browserContext;
   }
 
   async initialize(clientInfo: mcpServer.ClientInfo): Promise<void> {
     this._sessionLog = this._config.saveSession ? await SessionLog.create(this._config, clientInfo) : undefined;
-    this._context = new Context({
+    this._context = new Context(this.browserContext, {
       config: this._config,
-      browserContextFactory: this._browserContextFactory,
       sessionLog: this._sessionLog,
       clientInfo,
     });
   }
 
+  async dispose() {
+    await this._context?.dispose().catch(logUnhandledError);
+  }
+
   async listTools(): Promise<mcpServer.Tool[]> {
     return this._tools.map(tool => toMcpTool(tool.schema));
   }
@@ -82,8 +84,4 @@ export class BrowserServerBackend implements ServerBackend {
     }
     return responseObject;
   }
-
-  serverClosed() {
-    void this._context?.dispose().catch(logUnhandledError);
-  }
 }