cherry-pick(release-1.11): fix video saving

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);
   }
 

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();
   }
 

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 {

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();
   }
 

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);
+  });
+
 });