diff --git a/src/server/chromium/crPage.ts b/src/server/chromium/crPage.ts index b0a0fdbc75..86dbdbac8e 100644 --- a/src/server/chromium/crPage.ts +++ b/src/server/chromium/crPage.ts @@ -532,14 +532,25 @@ class FrameSession { if (frameSession && frameId !== this._targetId) { // This is a remote -> local frame transition. frameSession._swappedIn = true; - const frame = this._page._frameManager.frame(frameId)!; - this._page._frameManager.removeChildFramesRecursively(frame); + const frame = this._page._frameManager.frame(frameId); + // Frame or even a whole subtree may be already gone, because some ancestor did navigate. + if (frame) + this._page._frameManager.removeChildFramesRecursively(frame); + return; + } + if (parentFrameId && !this._page._frameManager.frame(parentFrameId)) { + // Parent frame may be gone already because some ancestor frame navigated and + // destroyed the whole subtree of some oopif, while oopif's process is still sending us events. + // Be careful to not confuse this with "main frame navigated cross-process" scenario + // where parentFrameId is null. return; } this._page._frameManager.frameAttached(frameId, parentFrameId); } _onFrameNavigated(framePayload: Protocol.Page.Frame, initial: boolean) { + if (!this._page._frameManager.frame(framePayload.id)) + return; // Subtree may be already gone because some ancestor navigation destroyed the oopif. this._page._frameManager.frameCommittedNewDocumentNavigation(framePayload.id, framePayload.url + (framePayload.urlFragment || ''), framePayload.name || '', framePayload.loaderId, initial); if (!initial) this._firstNonInitialNavigationCommittedFulfill(); @@ -609,7 +620,9 @@ class FrameSession { if (event.targetInfo.type === 'iframe') { // Frame id equals target id. const targetId = event.targetInfo.targetId; - const frame = this._page._frameManager.frame(targetId)!; + const frame = this._page._frameManager.frame(targetId); + if (!frame) + return; // Subtree may be already gone due to renderer/browser race. this._page._frameManager.removeChildFramesRecursively(frame); const frameSession = new FrameSession(this._crPage, session, targetId, this); this._crPage._sessions.set(targetId, frameSession); @@ -715,6 +728,8 @@ class FrameSession { } _onDialog(event: Protocol.Page.javascriptDialogOpeningPayload) { + if (!this._page._frameManager.frame(this._targetId)) + return; // Our frame/subtree may be gone already. this._page.emit(Page.Events.Dialog, new dialog.Dialog( this._page, event.type, @@ -749,10 +764,18 @@ class FrameSession { } async _onFileChooserOpened(event: Protocol.Page.fileChooserOpenedPayload) { - const frame = this._page._frameManager.frame(event.frameId)!; - const utilityContext = await frame._utilityContext(); - const handle = await this._adoptBackendNodeId(event.backendNodeId, utilityContext); - this._page._onFileChooserOpened(handle); + const frame = this._page._frameManager.frame(event.frameId); + if (!frame) + return; + let handle; + try { + const utilityContext = await frame._utilityContext(); + handle = await this._adoptBackendNodeId(event.backendNodeId, utilityContext); + } catch (e) { + // During async processing, frame/context may go away. We should not throw. + return; + } + await this._page._onFileChooserOpened(handle); } _onDownloadWillBegin(payload: Protocol.Page.downloadWillBeginPayload) { diff --git a/src/server/page.ts b/src/server/page.ts index 7abb26db77..b7a630858c 100644 --- a/src/server/page.ts +++ b/src/server/page.ts @@ -226,7 +226,13 @@ export class Page extends EventEmitter { } async _onFileChooserOpened(handle: dom.ElementHandle) { - const multiple = await handle.evaluate(element => !!(element as HTMLInputElement).multiple); + let multiple; + try { + multiple = await handle.evaluate(element => !!(element as HTMLInputElement).multiple); + } catch (e) { + // Frame/context may be gone during async processing. Do not throw. + return; + } if (!this.listenerCount(Page.Events.FileChooser)) { handle.dispose(); return; diff --git a/src/server/webkit/wkPage.ts b/src/server/webkit/wkPage.ts index 85699fc4f9..fccab1846b 100644 --- a/src/server/webkit/wkPage.ts +++ b/src/server/webkit/wkPage.ts @@ -559,9 +559,15 @@ export class WKPage implements PageDelegate { } private async _onFileChooserOpened(event: {frameId: Protocol.Network.FrameId, element: Protocol.Runtime.RemoteObject}) { - const context = await this._page._frameManager.frame(event.frameId)!._mainContext(); - const handle = context.createHandle(event.element).asElement()!; - this._page._onFileChooserOpened(handle); + let handle; + try { + const context = await this._page._frameManager.frame(event.frameId)!._mainContext(); + handle = context.createHandle(event.element).asElement()!; + } catch (e) { + // During async processing, frame/context may go away. We should not throw. + return; + } + await this._page._onFileChooserOpened(handle); } private static async _setEmulateMedia(session: WKSession, mediaType: types.MediaType | null, colorScheme: types.ColorScheme | null): Promise { diff --git a/test/page-set-input-files.spec.ts b/test/page-set-input-files.spec.ts index d41e8dc6e3..cf69149b62 100644 --- a/test/page-set-input-files.spec.ts +++ b/test/page-set-input-files.spec.ts @@ -115,6 +115,50 @@ it('should work when file input is not attached to DOM', async ({page, server}) expect(chooser).toBeTruthy(); }); +it('should not throw when filechooser belongs to iframe', (test, { browserName }) => { + test.skip(browserName === 'firefox', 'Firefox ignores filechooser from child frame'); +}, async ({page, server}) => { + await page.goto(server.PREFIX + '/frames/one-frame.html'); + const frame = page.mainFrame().childFrames()[0]; + await frame.setContent(` +
Click me
+ + `); + await Promise.all([ + page.waitForEvent('filechooser'), + frame.click('div') + ]); + await page.waitForFunction(() => (window as any).__done); +}); + +it('should not throw when frame is detached immediately', async ({page, server}) => { + await page.goto(server.PREFIX + '/frames/one-frame.html'); + const frame = page.mainFrame().childFrames()[0]; + await frame.setContent(` +
Click me
+ + `); + page.on('filechooser', () => {}); // To ensure we handle file choosers. + await frame.click('div'); + await page.waitForFunction(() => (window as any).__done); +}); + it('should work with CSP', async ({page, server}) => { server.setCSP('/empty.html', 'default-src "none"'); await page.goto(server.EMPTY_PAGE);