From 2393602e8ca5eb91f733dbe8ae9553c3f46e3493 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 5 May 2023 11:10:53 -0700 Subject: [PATCH] fix(reuse): ignore late bindings after dispatchers were disposed (#22838) When reusing the context, we first dispose all dispatchers, and then reset the page/context. If bindings are triggered during the reset, they try to send messages on disposed dispatchers. Since there is no way to unregister bindings, just ignore them after disposal. Fixes #22803. --- .../dispatchers/browserContextDispatcher.ts | 4 ++++ .../src/server/dispatchers/pageDispatcher.ts | 4 ++++ tests/library/browsercontext-reuse.spec.ts | 20 +++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts index 5ac9649588..22c2caf970 100644 --- a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts @@ -169,6 +169,10 @@ export class BrowserContextDispatcher extends Dispatcher { await this._context.exposeBinding(params.name, !!params.needsHandle, (source, ...args) => { + // When reusing the context, we might have some bindings called late enough, + // after context and page dispatchers have been disposed. + if (this._disposed) + return; const pageDispatcher = PageDispatcher.from(this, source.page); const binding = new BindingCallDispatcher(pageDispatcher, params.name, !!params.needsHandle, source, args); this._dispatchEvent('bindingCall', { binding }); diff --git a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts b/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts index 3e249692e2..2874730570 100644 --- a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts @@ -111,6 +111,10 @@ export class PageDispatcher extends Dispatcher { await this._page.exposeBinding(params.name, !!params.needsHandle, (source, ...args) => { + // When reusing the context, we might have some bindings called late enough, + // after context and page dispatchers have been disposed. + if (this._disposed) + return; const binding = new BindingCallDispatcher(this, params.name, !!params.needsHandle, source, args); this._dispatchEvent('bindingCall', { binding }); return binding.promise(); diff --git a/tests/library/browsercontext-reuse.spec.ts b/tests/library/browsercontext-reuse.spec.ts index f04cf28265..4d48404686 100644 --- a/tests/library/browsercontext-reuse.spec.ts +++ b/tests/library/browsercontext-reuse.spec.ts @@ -182,3 +182,23 @@ test('should not cache resources', async ({ reusedContext, server }) => { expect(requestCountMap.get('/simple.json')).toBe(2); } }); + +test('should ignore binding from beforeunload', async ({ reusedContext }) => { + test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/22803' }); + + let context = await reusedContext(); + + let called = false; + await context.exposeFunction('binding', () => called = true); + + let page = await context.newPage(); + await page.evaluate(() => { + window.addEventListener('beforeunload', () => window['binding']()); + }); + + context = await reusedContext(); + page = context.pages()[0]; + await page.setContent('hello'); + + expect(called).toBe(false); +});