Skip to content

Commit

Permalink
cherry-pick(release-1.11): fix video saving (#6671)
Browse files Browse the repository at this point in the history
- Cherry-Pick PR #6648 SHA b946437
- Cherry-Pick PR #6664 SHA 2ef47b9

References microsoft/playwright-java#432

Co-authored-by: Yury Semikhatsky <yurys@chromium.org>
  • Loading branch information
aslushnikov and yury-s committed May 20, 2021
1 parent 6a9939d commit c51bd43
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 6 deletions.
4 changes: 0 additions & 4 deletions src/server/browser.ts
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down
4 changes: 3 additions & 1 deletion src/server/chromium/crPage.ts
Expand Up @@ -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();
}

Expand Down
10 changes: 9 additions & 1 deletion src/server/firefox/ffBrowser.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions src/server/webkit/wkBrowser.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
}

Expand Down
28 changes: 28 additions & 0 deletions tests/screencast.spec.ts
Expand Up @@ -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);
});

});

0 comments on commit c51bd43

Please sign in to comment.