chore: cleanup more state upon disconnect (#36132)

packages/playwright-core/src/server/chromium/crCoverage.ts

@@ -115,8 +115,8 @@ class JSCoverage {
   }
 
   async stop(): Promise<channels.PageStopJSCoverageResult> {
-    assert(this._enabled, 'JSCoverage is not enabled');
-    this._enabled = false;
+    if (!this._enabled)
+      return { entries: [] };
     const [profileResponse] = await Promise.all([
       this._client.send('Profiler.takePreciseCoverage'),
       this._client.send('Profiler.stopPreciseCoverage'),
@@ -124,6 +124,7 @@ class JSCoverage {
       this._client.send('Debugger.disable'),
     ] as const);
     eventsHelper.removeEventListeners(this._eventListeners);
+    this._enabled = false;
 
     const coverage: channels.PageStopJSCoverageResult = { entries: [] };
     for (const entry of profileResponse.result) {
@@ -197,14 +198,15 @@ class CSSCoverage {
   }
 
   async stop(): Promise<channels.PageStopCSSCoverageResult> {
-    assert(this._enabled, 'CSSCoverage is not enabled');
-    this._enabled = false;
+    if (!this._enabled)
+      return { entries: [] };
     const ruleTrackingResponse = await this._client.send('CSS.stopRuleUsageTracking');
     await Promise.all([
       this._client.send('CSS.disable'),
       this._client.send('DOM.disable'),
     ]);
     eventsHelper.removeEventListeners(this._eventListeners);
+    this._enabled = false;
 
     // aggregate by styleSheetId
     const styleSheetIdToCoverage = new Map();

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

@@ -54,6 +54,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
   private _bindings: PageBinding[] = [];
   private _initScritps: InitScript[] = [];
   private _dialogHandler: (dialog: Dialog) => boolean;
+  private _clockPaused = false;
 
   static from(parentScope: DispatcherScope, context: BrowserContext): BrowserContextDispatcher {
     const result = parentScope.connection.existingDispatcher<BrowserContextDispatcher>(context);
@@ -343,10 +344,12 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
 
   async clockPauseAt(params: channels.BrowserContextClockPauseAtParams, metadata?: CallMetadata | undefined): Promise<channels.BrowserContextClockPauseAtResult> {
     await this._context.clock.pauseAt(params.timeString ?? params.timeNumber ?? 0);
+    this._clockPaused = true;
   }
 
   async clockResume(params: channels.BrowserContextClockResumeParams, metadata?: CallMetadata | undefined): Promise<channels.BrowserContextClockResumeResult> {
     await this._context.clock.resume();
+    this._clockPaused = false;
   }
 
   async clockRunFor(params: channels.BrowserContextClockRunForParams, metadata?: CallMetadata | undefined): Promise<channels.BrowserContextClockRunForResult> {
@@ -386,5 +389,8 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
     this._bindings = [];
     this._context.removeInitScripts(this._initScritps).catch(() => {});
     this._initScritps = [];
+    if (this._clockPaused)
+      this._context.clock.resume().catch(() => {});
+    this._clockPaused = false;
   }
 }

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

@@ -49,6 +49,8 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
   private _bindings: PageBinding[] = [];
   private _initScripts: InitScript[] = [];
   private _locatorHandlers = new Set<number>();
+  private _jsCoverageActive = false;
+  private _cssCoverageActive = false;
 
   static from(parentScope: BrowserContextDispatcher, page: Page): PageDispatcher {
     return PageDispatcher.fromNullable(parentScope, page)!;
@@ -229,7 +231,7 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
 
   async updateSubscription(params: channels.PageUpdateSubscriptionParams): Promise<void> {
     if (params.event === 'fileChooser')
-      await this._page.setFileChooserIntercepted(params.enabled);
+      await this._page.setFileChooserInterceptedBy(params.enabled, this);
     if (params.enabled)
       this._subscriptions.add(params.event);
     else
@@ -304,23 +306,29 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
   }
 
   async startJSCoverage(params: channels.PageStartJSCoverageParams, metadata: CallMetadata): Promise<void> {
+    this._jsCoverageActive = true;
     const coverage = this._page.coverage as CRCoverage;
     await coverage.startJSCoverage(params);
   }
 
   async stopJSCoverage(params: channels.PageStopJSCoverageParams, metadata: CallMetadata): Promise<channels.PageStopJSCoverageResult> {
     const coverage = this._page.coverage as CRCoverage;
-    return await coverage.stopJSCoverage();
+    const result = await coverage.stopJSCoverage();
+    this._jsCoverageActive = false;
+    return result;
   }
 
   async startCSSCoverage(params: channels.PageStartCSSCoverageParams, metadata: CallMetadata): Promise<void> {
+    this._cssCoverageActive = true;
     const coverage = this._page.coverage as CRCoverage;
     await coverage.startCSSCoverage(params);
   }
 
   async stopCSSCoverage(params: channels.PageStopCSSCoverageParams, metadata: CallMetadata): Promise<channels.PageStopCSSCoverageResult> {
     const coverage = this._page.coverage as CRCoverage;
-    return await coverage.stopCSSCoverage();
+    const result = await coverage.stopCSSCoverage();
+    this._cssCoverageActive = false;
+    return result;
   }
 
   _onFrameAttached(frame: Frame) {
@@ -343,6 +351,13 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
     for (const uid of this._locatorHandlers)
       this._page.unregisterLocatorHandler(uid);
     this._locatorHandlers.clear();
+    this._page.setFileChooserInterceptedBy(false, this).catch(() => {});
+    if (this._jsCoverageActive)
+      (this._page.coverage as CRCoverage).stopJSCoverage().catch(() => {});
+    this._jsCoverageActive = false;
+    if (this._cssCoverageActive)
+      (this._page.coverage as CRCoverage).stopCSSCoverage().catch(() => {});
+    this._cssCoverageActive = false;
   }
 }
 

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

@@ -151,7 +151,7 @@ export class Page extends SdkObject {
   private _emulatedSize: EmulatedSize | undefined;
   private _extraHTTPHeaders: types.HeadersArray | undefined;
   private _emulatedMedia: Partial<EmulatedMedia> = {};
-  private _interceptFileChooser = false;
+  private _fileChooserInterceptedBy = new Set<any>();
   private readonly _pageBindings = new Map<string, PageBinding>();
   initScripts: InitScript[] = [];
   readonly screenshotter: Screenshotter;
@@ -260,19 +260,16 @@ export class Page extends SdkObject {
 
     await this.setClientRequestInterceptor(undefined);
     await this.setServerRequestInterceptor(undefined);
-    await this.setFileChooserIntercepted(false);
     // Re-navigate once init scripts are gone.
     // TODO: we should have a timeout for `resetForReuse`.
     await this.mainFrame().goto(metadata, 'about:blank', { timeout: 0 });
     this._emulatedSize = undefined;
     this._emulatedMedia = {};
     this._extraHTTPHeaders = undefined;
-    this._interceptFileChooser = false;
 
     await Promise.all([
       this.delegate.updateEmulatedViewportSize(),
       this.delegate.updateEmulateMedia(),
-      this.delegate.updateFileChooserInterception(),
     ]);
 
     await this.delegate.resetForReuse();
@@ -744,13 +741,18 @@ export class Page extends SdkObject {
     }
   }
 
-  async setFileChooserIntercepted(enabled: boolean): Promise<void> {
-    this._interceptFileChooser = enabled;
-    await this.delegate.updateFileChooserInterception();
+  async setFileChooserInterceptedBy(enabled: boolean, by: any): Promise<void> {
+    const wasIntercepted = this.fileChooserIntercepted();
+    if (enabled)
+      this._fileChooserInterceptedBy.add(by);
+    else
+      this._fileChooserInterceptedBy.delete(by);
+    if (wasIntercepted !== this.fileChooserIntercepted())
+      await this.delegate.updateFileChooserInterception();
   }
 
   fileChooserIntercepted() {
-    return this._interceptFileChooser;
+    return this._fileChooserInterceptedBy.size > 0;
   }
 
   frameNavigatedToNewDocument(frame: frames.Frame) {

tests/library/multiclient.spec.ts

@@ -52,6 +52,12 @@ const test = playwrightTest.extend<ExtraFixtures>({
 test.slow(true, 'All connect tests are slow');
 test.skip(({ mode }) => mode.startsWith('service'));
 
+async function disconnect(page: Page) {
+  await page.context().browser().close();
+  // Give disconnect some time to cleanup.
+  await new Promise(f => setTimeout(f, 1000));
+}
+
 test('should connect two clients', async ({ connect, remoteServer, server }) => {
   const browserA = await connect(remoteServer.wsEndpoint());
   expect(browserA.contexts().length).toBe(0);
@@ -79,7 +85,8 @@ test('should connect two clients', async ({ connect, remoteServer, server }) =>
   await expect(pageB2).toHaveURL('/frames/frame.html');
 
   // Both contexts and pages should be still operational after any client disconnects.
-  await browserA.close();
+  await disconnect(pageA1);
+
   await expect(pageB1).toHaveURL(server.EMPTY_PAGE);
   await expect(pageB2).toHaveURL(server.PREFIX + '/frames/frame.html');
 });
@@ -109,20 +116,39 @@ test('should receive viewport size changes', async ({ twoPages }) => {
   await expect.poll(() => pageA.viewportSize()).toEqual({ width: 456, height: 567 });
 });
 
-test('should not allow parallel js coverage', async ({ twoPages, browserName }) => {
+test('should not allow parallel js coverage and cleanup upon disconnect', async ({ twoPages, browserName }) => {
   test.skip(browserName !== 'chromium');
+
   const { pageA, pageB } = twoPages;
   await pageA.coverage.startJSCoverage();
   const error = await pageB.coverage.startJSCoverage().catch(e => e);
   expect(error.message).toContain('JSCoverage is already enabled');
+
+  // Should cleanup coverage on disconnect and allow another client to start it.
+  await disconnect(pageA);
+  await pageB.coverage.startJSCoverage();
 });
 
 test('should not allow parallel css coverage', async ({ twoPages, browserName }) => {
   test.skip(browserName !== 'chromium');
+
   const { pageA, pageB } = twoPages;
   await pageA.coverage.startCSSCoverage();
   const error = await pageB.coverage.startCSSCoverage().catch(e => e);
   expect(error.message).toContain('CSSCoverage is already enabled');
+
+  // Should cleanup coverage on disconnect and allow another client to start it.
+  await disconnect(pageA);
+  await pageB.coverage.startCSSCoverage();
+});
+
+test('should unpause clock', async ({ twoPages }) => {
+  const { pageA, pageB } = twoPages;
+  await pageA.clock.install({ time: 1000 });
+  await pageA.clock.pauseAt(2000);
+  const promise = pageB.evaluate(() => new Promise(f => setTimeout(f, 1000)));
+  await disconnect(pageA);
+  await promise;
 });
 
 test('last emulateMedia wins', async ({ twoPages }) => {
@@ -153,8 +179,7 @@ test('should remove exposed bindings upon disconnect', async ({ twoPages }) => {
   await pageB.context().exposeBinding('contextBindingB', () => 'contextBindingBResult');
   expect(await pageA.evaluate(() => (window as any).contextBindingB())).toBe('contextBindingBResult');
 
-  await pageA.context().browser().close();
-  await new Promise(f => setTimeout(f, 1000)); // Give disconnect some time to cleanup.
+  await disconnect(pageA);
 
   expect(await pageB.evaluate(() => (window as any).pageBindingA)).toBe(undefined);
   expect(await pageB.evaluate(() => (window as any).contextBindingA)).toBe(undefined);
@@ -179,8 +204,7 @@ test('should remove init scripts upon disconnect', async ({ twoPages, server })
   expect(await pageA.evaluate(() => (window as any).pageValueB)).toBe('pageValueB');
   expect(await pageA.evaluate(() => (window as any).contextValueB)).toBe('contextValueB');
 
-  await pageB.context().browser().close();
-  await new Promise(f => setTimeout(f, 1000)); // Give disconnect some time to cleanup.
+  await disconnect(pageB);
 
   await pageA.goto(server.EMPTY_PAGE);
   expect(await pageA.evaluate(() => (window as any).pageValueB)).toBe(undefined);
@@ -216,7 +240,8 @@ test('should remove locator handlers upon disconnect', async ({ twoPages, server
     (window as any).setupAnnoyingInterstitial('mouseover', 1);
   });
 
-  await pageA.context().browser().close();
+  await disconnect(pageA);
+
   const error = await pageB.locator('#target').click({ timeout: 3000 }).catch(e => e);
   expect(error.message).toContain('Timeout 3000ms exceeded');
   expect(error.message).toContain('intercepts pointer events');