fix(chromium): close all javascript dialogs when closing context (#11614)

Fixes #11581

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

@@ -21,6 +21,7 @@ import { assert } from '../../utils/utils';
 import * as network from '../network';
 import { Page, PageBinding, PageDelegate, Worker } from '../page';
 import { Frame } from '../frames';
+import { Dialog } from '../dialog';
 import { ConnectionTransport } from '../transport';
 import * as types from '../types';
 import { ConnectionEvents, CRConnection, CRSession } from './crConnection';
@@ -471,6 +472,18 @@ export class CRBrowserContext extends BrowserContext {
 
   async _doClose() {
     assert(this._browserContextId);
+    // Headful chrome cannot dispose browser context with opened 'beforeunload'
+    // dialogs, so we should close all that are currently opened.
+    // We also won't get new ones since `Target.disposeBrowserContext` does not trigger
+    // beforeunload.
+    const openedBeforeUnloadDialogs: Dialog[] = [];
+    for (const crPage of this._browser._crPages.values()) {
+      if (crPage._browserContext !== this)
+        continue;
+      const dialogs = [...crPage._page._frameManager._openedDialogs].filter(dialog => dialog.type() === 'beforeunload');
+      openedBeforeUnloadDialogs.push(...dialogs);
+    }
+    await Promise.all(openedBeforeUnloadDialogs.map(dialog => dialog.dismiss()));
     await this._browser._session.send('Target.disposeBrowserContext', { browserContextId: this._browserContextId });
     this._browser._contexts.delete(this._browserContextId);
     for (const [targetId, serviceWorker] of this._browser._serviceWorkers) {

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

@@ -38,7 +38,7 @@ export class Dialog extends SdkObject {
     this._message = message;
     this._onHandle = onHandle;
     this._defaultValue = defaultValue || '';
-    this._page._frameManager.dialogDidOpen();
+    this._page._frameManager.dialogDidOpen(this);
   }
 
   type(): string {
@@ -56,14 +56,14 @@ export class Dialog extends SdkObject {
   async accept(promptText: string | undefined) {
     assert(!this._handled, 'Cannot accept dialog which is already handled!');
     this._handled = true;
-    this._page._frameManager.dialogWillClose();
+    this._page._frameManager.dialogWillClose(this);
     await this._onHandle(true, promptText);
   }
 
   async dismiss() {
     assert(!this._handled, 'Cannot dismiss dialog which is already handled!');
     this._handled = true;
-    this._page._frameManager.dialogWillClose();
+    this._page._frameManager.dialogWillClose(this);
     await this._onHandle(false);
   }
 }

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

@@ -22,6 +22,7 @@ import { helper } from './helper';
 import { eventsHelper, RegisteredListener } from '../utils/eventsHelper';
 import * as js from './javascript';
 import * as network from './network';
+import { Dialog } from './dialog';
 import { Page } from './page';
 import * as types from './types';
 import { BrowserContext } from './browserContext';
@@ -85,7 +86,7 @@ export class FrameManager {
   readonly _consoleMessageTags = new Map<string, ConsoleTagHandler>();
   readonly _signalBarriers = new Set<SignalBarrier>();
   private _webSockets = new Map<string, network.WebSocket>();
-  _dialogCounter = 0;
+  _openedDialogs: Set<Dialog> = new Set();
 
   constructor(page: Page) {
     this._page = page;
@@ -307,15 +308,15 @@ export class FrameManager {
     this._page._browserContext.emit(BrowserContext.Events.RequestFailed, request);
   }
 
-  dialogDidOpen() {
+  dialogDidOpen(dialog: Dialog) {
     // Any ongoing evaluations will be stalled until the dialog is closed.
     for (const frame of this._frames.values())
       frame._invalidateNonStallingEvaluations('JavaScript dialog interrupted evaluation');
-    this._dialogCounter++;
+    this._openedDialogs.add(dialog);
   }
 
-  dialogWillClose() {
-    this._dialogCounter--;
+  dialogWillClose(dialog: Dialog) {
+    this._openedDialogs.delete(dialog);
   }
 
   removeChildFramesRecursively(frame: Frame) {
@@ -508,7 +509,7 @@ export class Frame extends SdkObject {
   async nonStallingRawEvaluateInExistingMainContext(expression: string): Promise<any> {
     if (this._pendingDocument)
       throw new Error('Frame is currently attempting a navigation');
-    if (this._page._frameManager._dialogCounter)
+    if (this._page._frameManager._openedDialogs.size)
       throw new Error('Open JavaScript dialog prevents evaluation');
     const context = this._existingMainContext();
     if (!context)

tests/headful.spec.ts

@@ -35,6 +35,23 @@ it('should close browser with beforeunload page', async ({ browserType, server,
   await browserContext.close();
 });
 
+it('should close browsercontext with pending beforeunload dialog', async ({ server, browserType }) => {
+  const browser = await browserType.launch({ headless: false });
+  const context = await browser.newContext();
+  const page = await context.newPage();
+  await page.goto(server.PREFIX + '/beforeunload.html');
+  // We have to interact with a page so that 'beforeunload' handlers
+  // fire.
+  await page.click('body');
+  await Promise.all([
+    page.waitForEvent('dialog'),
+    page.close({ runBeforeUnload: true }),
+  ]);
+  await context.close();
+  await browser.close();
+});
+
+
 it('should not crash when creating second context', async ({ browserType }) => {
   const browser = await browserType.launch({ headless: false });
   {