Skip to content

Commit

Permalink
fix: do not report errored pages after context closure (#4346)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dgozman authored Nov 5, 2020
1 parent 4cb5214 commit e942138
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 3 deletions.
4 changes: 4 additions & 0 deletions src/server/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
7 changes: 6 additions & 1 deletion src/server/chromium/crBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
7 changes: 6 additions & 1 deletion src/server/firefox/ffBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion src/server/webkit/wkBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit e942138

Please sign in to comment.