diff --git a/src/server/browser.ts b/src/server/browser.ts index e0a991b5ee..b05919d503 100644 --- a/src/server/browser.ts +++ b/src/server/browser.ts @@ -24,7 +24,6 @@ import { RecentLogsCollector } from '../utils/debugLogger'; import * as registry from '../utils/registry'; import { SdkObject } from './instrumentation'; import { Artifact } from './artifact'; -import { kBrowserClosedError } from '../utils/errors'; export interface BrowserProcess { onclose?: ((exitCode: number | null, signal: string | null) => void); @@ -120,9 +119,6 @@ export abstract class Browser extends SdkObject { context._browserClosed(); if (this._defaultContext) this._defaultContext._browserClosed(); - for (const video of this._idToVideo.values()) - video.artifact.reportFinished(kBrowserClosedError); - this._idToVideo.clear(); this.emit(Browser.Events.Disconnected); } diff --git a/src/server/chromium/crPage.ts b/src/server/chromium/crPage.ts index d90d438075..324856cd6f 100644 --- a/src/server/chromium/crPage.ts +++ b/src/server/chromium/crPage.ts @@ -900,9 +900,11 @@ class FrameSession { this._screencastId = null; const recorder = this._videoRecorder!; this._videoRecorder = null; - const video = this._crPage._browserContext._browser._takeVideo(screencastId); await this._stopScreencast(recorder); await recorder.stop().catch(() => {}); + // Keep the video artifact in the map utntil encoding is fully finished, if the context + // starts closing before the video is fully written to disk it will wait for it. + const video = this._crPage._browserContext._browser._takeVideo(screencastId); video?.reportFinished(); } diff --git a/src/server/firefox/ffBrowser.ts b/src/server/firefox/ffBrowser.ts index 7c1baaaa74..7a7751c16f 100644 --- a/src/server/firefox/ffBrowser.ts +++ b/src/server/firefox/ffBrowser.ts @@ -15,6 +15,7 @@ * limitations under the License. */ +import { kBrowserClosedError } from '../../utils/errors'; import { assert } from '../../utils/utils'; import { Browser, BrowserOptions } from '../browser'; import { assertBrowserContextIsNotOwned, BrowserContext, validateBrowserContextOptions, verifyGeolocation } from '../browserContext'; @@ -56,7 +57,7 @@ export class FFBrowser extends Browser { this._connection = connection; this._ffPages = new Map(); this._contexts = new Map(); - this._connection.on(ConnectionEvents.Disconnected, () => this._didClose()); + this._connection.on(ConnectionEvents.Disconnected, () => this._onDisconnect()); this._connection.on('Browser.attachedToTarget', this._onAttachedToTarget.bind(this)); this._connection.on('Browser.detachedFromTarget', this._onDetachedFromTarget.bind(this)); this._connection.on('Browser.downloadCreated', this._onDownloadCreated.bind(this)); @@ -136,6 +137,13 @@ export class FFBrowser extends Browser { _onScreencastFinished(payload: Protocol.Browser.screencastFinishedPayload) { this._takeVideo(payload.screencastId)?.reportFinished(); } + + _onDisconnect() { + for (const video of this._idToVideo.values()) + video.artifact.reportFinished(kBrowserClosedError); + this._idToVideo.clear(); + this._didClose(); + } } export class FFBrowserContext extends BrowserContext { diff --git a/src/server/webkit/wkBrowser.ts b/src/server/webkit/wkBrowser.ts index 679650569e..019dad039f 100644 --- a/src/server/webkit/wkBrowser.ts +++ b/src/server/webkit/wkBrowser.ts @@ -26,6 +26,7 @@ import * as types from '../types'; import { Protocol } from './protocol'; import { kPageProxyMessageReceived, PageProxyMessageReceivedPayload, WKConnection, WKSession } from './wkConnection'; import { WKPage } from './wkPage'; +import { kBrowserClosedError } from '../../utils/errors'; const DEFAULT_USER_AGENT = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.2 Safari/605.1.15'; const BROWSER_VERSION = '14.2'; @@ -72,6 +73,9 @@ export class WKBrowser extends Browser { _onDisconnect() { for (const wkPage of this._wkPages.values()) wkPage.dispose(true); + for (const video of this._idToVideo.values()) + video.artifact.reportFinished(kBrowserClosedError); + this._idToVideo.clear(); this._didClose(); } diff --git a/tests/screencast.spec.ts b/tests/screencast.spec.ts index 15c9ea6ec2..ce5edd885d 100644 --- a/tests/screencast.spec.ts +++ b/tests/screencast.spec.ts @@ -615,4 +615,32 @@ it.describe('screencast', () => { const saveResult = await page.video().saveAs(file).catch(e => e); expect(saveResult.message).toContain('rowser has been closed'); }); + + it('should wait for video to finish if page was closed', async ({browserType, browserOptions, contextOptions}, testInfo) => { + const size = { width: 320, height: 240 }; + const browser = await browserType.launch(browserOptions); + + const videoDir = testInfo.outputPath(''); + const context = await browser.newContext({ + ...contextOptions, + recordVideo: { + dir: videoDir, + size, + }, + viewport: size, + }); + + const page = await context.newPage(); + await new Promise(r => setTimeout(r, 1000)); + await page.close(); + await context.close(); + await browser.close(); + + const videoFiles = findVideos(videoDir); + expect(videoFiles.length).toBe(1); + const videoPlayer = new VideoPlayer(videoFiles[0]); + expect(videoPlayer.videoWidth).toBe(320); + expect(videoPlayer.videoHeight).toBe(240); + }); + });