fix(evaluate): reject all context operations when frame detaches (#9987)

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

@@ -192,7 +192,7 @@ export class CRBrowser extends Browser {
     const serviceWorker = this._serviceWorkers.get(targetId);
     if (serviceWorker) {
       this._serviceWorkers.delete(targetId);
-      serviceWorker.emit(Worker.Events.Close);
+      serviceWorker.didClose();
       return;
     }
   }
@@ -471,7 +471,7 @@ export class CRBrowserContext extends BrowserContext {
       // asynchronously and we get detached from them later.
       // To avoid the wrong order of notifications, we manually fire
       // "close" event here and forget about the serivce worker.
-      serviceWorker.emit(Worker.Events.Close);
+      serviceWorker.didClose();
       this._browser._serviceWorkers.delete(targetId);
     }
   }

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

@@ -663,6 +663,7 @@ class FrameSession {
     else if (contextPayload.name === UTILITY_WORLD_NAME)
       worldName = 'utility';
     const context = new dom.FrameExecutionContext(delegate, frame, worldName);
+    (context as any)[contextDelegateSymbol] = delegate;
     if (worldName)
       frame._contextCreated(worldName, context);
     this._contextIdToContext.set(contextPayload.id, context);
@@ -1135,7 +1136,7 @@ class FrameSession {
   async _adoptBackendNodeId(backendNodeId: Protocol.DOM.BackendNodeId, to: dom.FrameExecutionContext): Promise<dom.ElementHandle> {
     const result = await this._client._sendMayFail('DOM.resolveNode', {
       backendNodeId,
-      executionContextId: (to._delegate as CRExecutionContext)._contextId,
+      executionContextId: ((to as any)[contextDelegateSymbol] as CRExecutionContext)._contextId,
     });
     if (!result || result.object.subtype === 'null')
       throw new Error(dom.kUnableToAdoptErrorMessage);
@@ -1167,3 +1168,5 @@ async function emulateTimezone(session: CRSession, timezoneId: string) {
     throw exception;
   }
 }
+
+const contextDelegateSymbol = Symbol('delegate');

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

@@ -95,7 +95,7 @@ export class FrameExecutionContext extends js.ExecutionContext {
         );
         })();
       `;
-      this._injectedScriptPromise = this._delegate.rawEvaluateHandle(source).then(objectId => new js.JSHandle(this, 'object', undefined, objectId));
+      this._injectedScriptPromise = this.rawEvaluateHandle(source).then(objectId => new js.JSHandle(this, 'object', undefined, objectId));
     }
     return this._injectedScriptPromise;
   }

packages/playwright-core/src/server/firefox/ffPage.ts

@@ -163,6 +163,7 @@ export class FFPage implements PageDelegate {
     else if (!auxData.name)
       worldName = 'main';
     const context = new dom.FrameExecutionContext(delegate, frame, worldName);
+    (context as any)[contextDelegateSymbol] = delegate;
     if (worldName)
       frame._contextCreated(worldName, context);
     this._contextIdToContext.set(executionContextId, context);
@@ -536,7 +537,7 @@ export class FFPage implements PageDelegate {
     const result = await this._session.send('Page.adoptNode', {
       frameId: handle._context.frame._id,
       objectId: handle._objectId,
-      executionContextId: (to._delegate as FFExecutionContext)._executionContextId
+      executionContextId: ((to as any)[contextDelegateSymbol] as FFExecutionContext)._executionContextId
     });
     if (!result.remoteObject)
       throw new Error(dom.kUnableToAdoptErrorMessage);
@@ -570,3 +571,5 @@ export class FFPage implements PageDelegate {
 function webSocketId(frameId: string, wsid: string): string {
   return `${frameId}---${wsid}`;
 }
+
+const contextDelegateSymbol = Symbol('delegate');

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

@@ -35,8 +35,7 @@ import type { ElementStateWithoutStable, FrameExpectParams, InjectedScriptPoll,
 import { isSessionClosedError } from './common/protocolError';
 
 type ContextData = {
-  contextPromise: Promise<dom.FrameExecutionContext>;
-  contextResolveCallback: (c: dom.FrameExecutionContext) => void;
+  contextPromise: ManualPromise<dom.FrameExecutionContext | Error>;
   context: dom.FrameExecutionContext | null;
   rerunnableTasks: Set<RerunnableTask<any>>;
 };
@@ -445,8 +444,8 @@ export class Frame extends SdkObject {
 
     this._detachedPromise = new Promise<void>(x => this._detachedCallback = x);
 
-    this._contextData.set('main', { contextPromise: new Promise(() => {}), contextResolveCallback: () => {}, context: null, rerunnableTasks: new Set() });
-    this._contextData.set('utility', { contextPromise: new Promise(() => {}), contextResolveCallback: () => {}, context: null, rerunnableTasks: new Set() });
+    this._contextData.set('main', { contextPromise: new ManualPromise(), context: null, rerunnableTasks: new Set() });
+    this._contextData.set('utility', { contextPromise: new ManualPromise(), context: null, rerunnableTasks: new Set() });
     this._setContext('main', null);
     this._setContext('utility', null);
 
@@ -662,9 +661,11 @@ export class Frame extends SdkObject {
   }
 
   _context(world: types.World): Promise<dom.FrameExecutionContext> {
-    if (this._detached)
-      throw new Error(`Execution Context is not available in detached frame "${this.url()}" (are you trying to evaluate?)`);
-    return this._contextData.get(world)!.contextPromise;
+    return this._contextData.get(world)!.contextPromise.then(contextOrError => {
+      if (contextOrError instanceof js.ExecutionContext)
+        return contextOrError;
+      throw contextOrError;
+    });
   }
 
   _mainContext(): Promise<dom.FrameExecutionContext> {
@@ -1250,9 +1251,13 @@ export class Frame extends SdkObject {
     this._stopNetworkIdleTimer();
     this._detached = true;
     this._detachedCallback();
+    const error = new Error('Frame was detached');
     for (const data of this._contextData.values()) {
+      if (data.context)
+        data.context.contextDestroyed(error);
+      data.contextPromise.resolve(error);
       for (const rerunnableTask of data.rerunnableTasks)
-        rerunnableTask.terminate(new Error('waitForFunction failed: frame got detached.'));
+        rerunnableTask.terminate(error);
     }
     if (this._parentFrame)
       this._parentFrame._childFrames.delete(this);
@@ -1339,13 +1344,11 @@ export class Frame extends SdkObject {
     const data = this._contextData.get(world)!;
     data.context = context;
     if (context) {
-      data.contextResolveCallback.call(null, context);
+      data.contextPromise.resolve(context);
       for (const rerunnableTask of data.rerunnableTasks)
         rerunnableTask.rerun(context);
     } else {
-      data.contextPromise = new Promise(fulfill => {
-        data.contextResolveCallback = fulfill;
-      });
+      data.contextPromise = new ManualPromise();
     }
   }
 
@@ -1354,12 +1357,19 @@ export class Frame extends SdkObject {
     // In case of multiple sessions to the same target, there's a race between
     // connections so we might end up creating multiple isolated worlds.
     // We can use either.
-    if (data.context)
+    if (data.context) {
+      data.context.contextDestroyed(new Error('Execution context was destroyed, most likely because of a navigation'));
       this._setContext(world, null);
+    }
     this._setContext(world, context);
   }
 
   _contextDestroyed(context: dom.FrameExecutionContext) {
+    // Sometimes we get this after detach, in which case we should not reset
+    // our already destroyed contexts to something that will never resolve.
+    if (this._detached)
+      return;
+    context.contextDestroyed(new Error('Execution context was destroyed, most likely because of a navigation'));
     for (const [world, data] of this._contextData) {
       if (data.context === context)
         this._setContext(world, null);

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

@@ -19,6 +19,7 @@ import * as utilityScriptSource from '../generated/utilityScriptSource';
 import { serializeAsCallArgument } from './common/utilityScriptSerializers';
 import type UtilityScript from './injected/utilityScript';
 import { SdkObject } from './instrumentation';
+import { ManualPromise } from '../utils/async';
 
 export type ObjectId = string;
 export type RemoteObject = {
@@ -53,14 +54,54 @@ export interface ExecutionContextDelegate {
 }
 
 export class ExecutionContext extends SdkObject {
-  readonly _delegate: ExecutionContextDelegate;
+  private _delegate: ExecutionContextDelegate;
   private _utilityScriptPromise: Promise<JSHandle> | undefined;
+  private _destroyedPromise = new ManualPromise<Error>();
 
   constructor(parent: SdkObject, delegate: ExecutionContextDelegate) {
     super(parent, 'execution-context');
     this._delegate = delegate;
   }
 
+  contextDestroyed(error: Error) {
+    this._destroyedPromise.resolve(error);
+  }
+
+  private _raceAgainstContextDestroyed<T>(promise: Promise<T>): Promise<T> {
+    return Promise.race([
+      this._destroyedPromise.then(e => { throw e; }),
+      promise,
+    ]);
+  }
+
+  rawEvaluateJSON(expression: string): Promise<any> {
+    return this._raceAgainstContextDestroyed(this._delegate.rawEvaluateJSON(expression));
+  }
+
+  rawEvaluateHandle(expression: string): Promise<ObjectId> {
+    return this._raceAgainstContextDestroyed(this._delegate.rawEvaluateHandle(expression));
+  }
+
+  rawCallFunctionNoReply(func: Function, ...args: any[]): void {
+    this._delegate.rawCallFunctionNoReply(func, ...args);
+  }
+
+  evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: JSHandle<any>, values: any[], objectIds: ObjectId[]): Promise<any> {
+    return this._raceAgainstContextDestroyed(this._delegate.evaluateWithArguments(expression, returnByValue, utilityScript, values, objectIds));
+  }
+
+  getProperties(context: ExecutionContext, objectId: ObjectId): Promise<Map<string, JSHandle>> {
+    return this._raceAgainstContextDestroyed(this._delegate.getProperties(context, objectId));
+  }
+
+  createHandle(remoteObject: RemoteObject): JSHandle {
+    return this._delegate.createHandle(this, remoteObject);
+  }
+
+  releaseHandle(objectId: ObjectId): Promise<void> {
+    return this._delegate.releaseHandle(objectId);
+  }
+
   async waitForSignalsCreatedBy<T>(action: () => Promise<T>): Promise<T> {
     return action();
   }
@@ -76,19 +117,11 @@ export class ExecutionContext extends SdkObject {
         ${utilityScriptSource.source}
         return new pwExport();
       })();`;
-      this._utilityScriptPromise = this._delegate.rawEvaluateHandle(source).then(objectId => new JSHandle(this, 'object', undefined, objectId));
+      this._utilityScriptPromise = this._raceAgainstContextDestroyed(this._delegate.rawEvaluateHandle(source).then(objectId => new JSHandle(this, 'object', undefined, objectId)));
     }
     return this._utilityScriptPromise;
   }
 
-  createHandle(remoteObject: RemoteObject): JSHandle {
-    return this._delegate.createHandle(this, remoteObject);
-  }
-
-  async rawEvaluateJSON(expression: string): Promise<any> {
-    return await this._delegate.rawEvaluateJSON(expression);
-  }
-
   async doSlowMo() {
     // overridden in FrameExecutionContext
   }
@@ -113,7 +146,7 @@ export class JSHandle<T = any> extends SdkObject {
   }
 
   callFunctionNoReply(func: Function, arg: any) {
-    this._context._delegate.rawCallFunctionNoReply(func, this, arg);
+    this._context.rawCallFunctionNoReply(func, this, arg);
   }
 
   async evaluate<R, Arg>(pageFunction: FuncOn<T, Arg, R>, arg?: Arg): Promise<R> {
@@ -145,7 +178,7 @@ export class JSHandle<T = any> extends SdkObject {
   async getProperties(): Promise<Map<string, JSHandle>> {
     if (!this._objectId)
       return new Map();
-    return this._context._delegate.getProperties(this._context, this._objectId);
+    return this._context.getProperties(this._context, this._objectId);
   }
 
   rawValue() {
@@ -157,7 +190,7 @@ export class JSHandle<T = any> extends SdkObject {
       return this._value;
     const utilityScript = await this._context.utilityScript();
     const script = `(utilityScript, ...args) => utilityScript.jsonValue(...args)`;
-    return this._context._delegate.evaluateWithArguments(script, true, utilityScript, [true], [this._objectId]);
+    return this._context.evaluateWithArguments(script, true, utilityScript, [true], [this._objectId]);
   }
 
   asElement(): dom.ElementHandle | null {
@@ -169,7 +202,7 @@ export class JSHandle<T = any> extends SdkObject {
       return;
     this._disposed = true;
     if (this._objectId)
-      this._context._delegate.releaseHandle(this._objectId).catch(e => {});
+      this._context.releaseHandle(this._objectId).catch(e => {});
   }
 
   override toString(): string {
@@ -232,7 +265,7 @@ export async function evaluateExpression(context: ExecutionContext, returnByValu
 
   const script = `(utilityScript, ...args) => utilityScript.evaluate(...args)`;
   try {
-    return await context._delegate.evaluateWithArguments(script, returnByValue, utilityScript, utilityScriptValues, utilityScriptObjectIds);
+    return await context.evaluateWithArguments(script, returnByValue, utilityScript, utilityScriptValues, utilityScriptObjectIds);
   } finally {
     toDispose.map(handlePromise => handlePromise.then(handle => handle.dispose()));
   }

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

@@ -464,13 +464,13 @@ export class Page extends SdkObject {
     const worker = this._workers.get(workerId);
     if (!worker)
       return;
-    worker.emit(Worker.Events.Close, worker);
+    worker.didClose();
     this._workers.delete(workerId);
   }
 
   _clearWorkers() {
     for (const [workerId, worker] of this._workers) {
-      worker.emit(Worker.Events.Close, worker);
+      worker.didClose();
       this._workers.delete(workerId);
     }
   }
@@ -547,6 +547,12 @@ export class Worker extends SdkObject {
     return this._url;
   }
 
+  didClose() {
+    if (this._existingExecutionContext)
+      this._existingExecutionContext.contextDestroyed(new Error('Worker was closed'));
+    this.emit(Worker.Events.Close, this);
+  }
+
   async evaluateExpression(expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
     return js.evaluateExpression(await this._executionContextPromise, true /* returnByValue */, expression, isFunction, arg);
   }

packages/playwright-core/src/server/webkit/wkExecutionContext.ts

@@ -24,19 +24,10 @@ import { isSessionClosedError } from '../common/protocolError';
 export class WKExecutionContext implements js.ExecutionContextDelegate {
   private readonly _session: WKSession;
   readonly _contextId: number | undefined;
-  private _contextDestroyedCallback: () => void = () => {};
-  private readonly _executionContextDestroyedPromise: Promise<unknown>;
 
   constructor(session: WKSession, contextId: number | undefined) {
     this._session = session;
     this._contextId = contextId;
-    this._executionContextDestroyedPromise = new Promise<void>((resolve, reject) => {
-      this._contextDestroyedCallback = resolve;
-    });
-  }
-
-  _dispose() {
-    this._contextDestroyedCallback();
   }
 
   async rawEvaluateJSON(expression: string): Promise<any> {
@@ -81,21 +72,18 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
 
   async evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: js.JSHandle<any>, values: any[], objectIds: string[]): Promise<any> {
     try {
-      const response = await Promise.race([
-        this._executionContextDestroyedPromise.then(() => { throw new Error(contextDestroyedError); }),
-        this._session.send('Runtime.callFunctionOn', {
-          functionDeclaration: expression,
-          objectId: utilityScript._objectId!,
-          arguments: [
-            { objectId: utilityScript._objectId },
-            ...values.map(value => ({ value })),
-            ...objectIds.map(objectId => ({ objectId })),
-          ],
-          returnByValue,
-          emulateUserGesture: true,
-          awaitPromise: true
-        })
-      ]);
+      const response = await this._session.send('Runtime.callFunctionOn', {
+        functionDeclaration: expression,
+        objectId: utilityScript._objectId!,
+        arguments: [
+          { objectId: utilityScript._objectId },
+          ...values.map(value => ({ value })),
+          ...objectIds.map(objectId => ({ objectId })),
+        ],
+        returnByValue,
+        emulateUserGesture: true,
+        awaitPromise: true
+      });
       if (response.wasThrown)
         throw new js.JavaScriptErrorInEvaluate(response.result.description);
       if (returnByValue)
@@ -130,8 +118,6 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
   }
 }
 
-const contextDestroyedError = 'Execution context was destroyed.';
-
 function potentiallyUnserializableValue(remoteObject: Protocol.Runtime.RemoteObject): any {
   const value = remoteObject.value;
   const isUnserializable = remoteObject.type === 'number' && ['NaN', '-Infinity', 'Infinity', '-0'].includes(remoteObject.description!);

packages/playwright-core/src/server/webkit/wkPage.ts

@@ -452,7 +452,6 @@ export class WKPage implements PageDelegate {
   private _removeContextsForFrame(frame: frames.Frame, notifyFrame: boolean) {
     for (const [contextId, context] of this._contextIdToContext) {
       if (context.frame === frame) {
-        (context._delegate as WKExecutionContext)._dispose();
         this._contextIdToContext.delete(contextId);
         if (notifyFrame)
           frame._contextDestroyed(context);
@@ -473,6 +472,7 @@ export class WKPage implements PageDelegate {
     else if (contextPayload.type === 'user' && contextPayload.name === UTILITY_WORLD_NAME)
       worldName = 'utility';
     const context = new dom.FrameExecutionContext(delegate, frame, worldName);
+    (context as any)[contextDelegateSymbol] = delegate;
     if (worldName)
       frame._contextCreated(worldName, context);
     if (contextPayload.type === 'normal' && frame === this._page.mainFrame())
@@ -902,7 +902,7 @@ export class WKPage implements PageDelegate {
   async adoptElementHandle<T extends Node>(handle: dom.ElementHandle<T>, to: dom.FrameExecutionContext): Promise<dom.ElementHandle<T>> {
     const result = await this._session.sendMayFail('DOM.resolveNode', {
       objectId: handle._objectId,
-      executionContextId: (to._delegate as WKExecutionContext)._contextId
+      executionContextId: ((to as any)[contextDelegateSymbol] as WKExecutionContext)._contextId
     });
     if (!result || result.object.subtype === 'null')
       throw new Error(dom.kUnableToAdoptErrorMessage);
@@ -1139,3 +1139,5 @@ function isLoadedSecurely(url: string, timing: network.ResourceTiming) {
     return true;
   } catch (_) {}
 }
+
+const contextDelegateSymbol = Symbol('delegate');

tests/page/frame-evaluate.spec.ts

@@ -113,7 +113,7 @@ it('should throw for detached frames', async ({ page, server }) => {
   await detachFrame(page, 'frame1');
   let error = null;
   await frame1.evaluate(() => 7 * 8).catch(e => error = e);
-  expect(error.message).toContain('Execution Context is not available in detached frame');
+  expect(error.message).toContain('frame.evaluate: Frame was detached');
 });
 
 it('should be isolated between frames', async ({ page, server }) => {

tests/page/page-evaluate.spec.ts

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { attachFrame, detachFrame } from '../config/utils';
 import { test as it, expect } from './pageTest';
 
 it('should work', async ({ page }) => {
@@ -570,3 +571,13 @@ it('should not use toJSON in jsonValue', async ({ page }) => {
 it('should not expose the injected script export', async ({ page }) => {
   expect(await page.evaluate('typeof pwExport === "undefined"')).toBe(true);
 });
+
+it('should throw when frame is detached', async ({ page, server }) => {
+  await attachFrame(page, 'frame1', server.EMPTY_PAGE);
+  const frame = page.frames()[1];
+  const promise = frame.evaluate(() => new Promise<void>(() => {})).catch(e => e);
+  await detachFrame(page, 'frame1');
+  const error = await promise;
+  expect(error).toBeTruthy();
+  expect(error.message).toMatch(/frame.evaluate: (Frame was detached|Execution context was destroyed)/);
+});

tests/page/page-wait-for-function.spec.ts

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { attachFrame, detachFrame } from '../config/utils';
 import { test as it, expect } from './pageTest';
 
 it('should timeout', async ({ page }) => {
@@ -264,3 +265,13 @@ it('should not be called after finishing unsuccessfully', async ({ page, server
 
   expect(messages.join('|')).toBe('waitForFunction1|waitForFunction2|waitForFunction3');
 });
+
+it('should throw when frame is detached', async ({ page, server }) => {
+  await attachFrame(page, 'frame1', server.EMPTY_PAGE);
+  const frame = page.frames()[1];
+  const promise = frame.waitForFunction(() => false).catch(e => e);
+  await detachFrame(page, 'frame1');
+  const error = await promise;
+  expect(error).toBeTruthy();
+  expect(error.message).toMatch(/frame.waitForFunction: (Frame was detached|Execution context was destroyed)/);
+});

tests/page/page-wait-for-selector-1.spec.ts

@@ -81,7 +81,7 @@ it('elementHandle.waitForSelector should throw on navigation', async ({ page, se
     await page.evaluate(() => 1);
   await page.goto(server.EMPTY_PAGE);
   const error = await promise;
-  expect(error.message).toContain('Execution context was destroyed, most likely because of a navigation');
+  expect(error.message).toContain('Execution context was destroyed');
 });
 
 it('should work with removed MutationObserver', async ({ page, server }) => {
@@ -242,5 +242,5 @@ it('should throw when frame is detached', async ({ page, server }) => {
   await detachFrame(page, 'frame1');
   await waitPromise;
   expect(waitError).toBeTruthy();
-  expect(waitError.message).toContain('waitForFunction failed: frame got detached.');
+  expect(waitError.message).toContain('frame.waitForSelector: Frame was detached');
 });

tests/page/page-wait-for-selector-2.spec.ts

@@ -236,7 +236,7 @@ it('should throw when frame is detached xpath', async ({ page, server }) => {
   await detachFrame(page, 'frame1');
   await waitPromise;
   expect(waitError).toBeTruthy();
-  expect(waitError.message).toContain('waitForFunction failed: frame got detached.');
+  expect(waitError.message).toContain('frame.waitForSelector: Frame was detached');
 });
 
 it('should return the element handle xpath', async ({ page, server }) => {

tests/page/workers.spec.ts

@@ -41,7 +41,7 @@ it('should emit created and destroyed events', async function({ page }) {
   await page.evaluate(workerObj => workerObj.terminate(), workerObj);
   expect(await workerDestroyedPromise).toBe(worker);
   const error = await workerThisObj.getProperty('self').catch(error => error);
-  expect(error.message).toContain('Target closed');
+  expect(error.message).toMatch(/jSHandle.getProperty: (Worker was closed|Target closed)/);
 });
 
 it('should report console logs', async function({ page }) {

tests/trace-viewer/trace-viewer.spec.ts

@@ -174,7 +174,7 @@ test.beforeAll(async function recordTrace({ browser, browserName, browserType, s
 
 test('should show empty trace viewer', async ({ showTraceViewer }, testInfo) => {
   const traceViewer = await showTraceViewer(testInfo.outputPath());
-  expect(await traceViewer.page.title()).toBe('Playwright Trace Viewer');
+  await expect(traceViewer.page).toHaveTitle('Playwright Trace Viewer');
 });
 
 test('should open simple trace viewer', async ({ showTraceViewer }) => {