From e9421389136eebbb72df619fb028cc6eba46294f Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 4 Nov 2020 16:23:38 -0800 Subject: [PATCH] fix: do not report errored pages after context closure (#4346) Consider the following sequence: - page opens a popup; - popup target is attached, we start initializing it; - user calls browser.close(); - browser is closed, and popup initialization fails; - we report "errored page" on the already closed context; - RPC client cannot make sense of this: "Cannot find parent object BrowserContext@guid to create Frame@guid" This issue was revealed during Firefox pipe migration. --- src/server/browserContext.ts | 4 ++++ src/server/chromium/crBrowser.ts | 7 ++++++- src/server/firefox/ffBrowser.ts | 7 ++++++- src/server/webkit/wkBrowser.ts | 7 ++++++- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/server/browserContext.ts b/src/server/browserContext.ts index e0368260b5..0459aa2b06 100644 --- a/src/server/browserContext.ts +++ b/src/server/browserContext.ts @@ -267,6 +267,10 @@ export abstract class BrowserContext extends EventEmitter { await this._doUpdateRequestInterception(); } + isClosingOrClosed() { + return this._closedStatus !== 'open'; + } + async close() { if (this._closedStatus === 'open') { this._closedStatus = 'closing'; diff --git a/src/server/chromium/crBrowser.ts b/src/server/chromium/crBrowser.ts index 7630ac4637..b6ea6c9652 100644 --- a/src/server/chromium/crBrowser.ts +++ b/src/server/chromium/crBrowser.ts @@ -164,8 +164,13 @@ export class CRBrowser extends Browser { this._crPages.set(targetInfo.targetId, crPage); crPage.pageOrError().then(pageOrError => { const page = crPage._page; - if (pageOrError instanceof Error) + if (pageOrError instanceof Error) { + // Initialization error could have happened because of + // context/browser closure. Just ignore the page. + if (context!.isClosingOrClosed()) + return; page._setIsError(); + } context!.emit(BrowserContext.Events.Page, page); if (opener) { opener.pageOrError().then(openerPage => { diff --git a/src/server/firefox/ffBrowser.ts b/src/server/firefox/ffBrowser.ts index db00b03a38..1fed0d5628 100644 --- a/src/server/firefox/ffBrowser.ts +++ b/src/server/firefox/ffBrowser.ts @@ -108,8 +108,13 @@ export class FFBrowser extends Browser { ffPage.pageOrError().then(async pageOrError => { const page = ffPage._page; - if (pageOrError instanceof Error) + if (pageOrError instanceof Error) { + // Initialization error could have happened because of + // context/browser closure. Just ignore the page. + if (context.isClosingOrClosed()) + return; page._setIsError(); + } context.emit(BrowserContext.Events.Page, page); if (!opener) return; diff --git a/src/server/webkit/wkBrowser.ts b/src/server/webkit/wkBrowser.ts index 466b6df3b1..d098ef688e 100644 --- a/src/server/webkit/wkBrowser.ts +++ b/src/server/webkit/wkBrowser.ts @@ -156,8 +156,13 @@ export class WKBrowser extends Browser { wkPage.pageOrError().then(async pageOrError => { const page = wkPage._page; - if (pageOrError instanceof Error) + if (pageOrError instanceof Error) { + // Initialization error could have happened because of + // context/browser closure. Just ignore the page. + if (context!.isClosingOrClosed()) + return; page._setIsError(); + } context!.emit(BrowserContext.Events.Page, page); if (!opener) return;