diff --git a/packages/playwright-core/src/client/artifact.ts b/packages/playwright-core/src/client/artifact.ts index 0eb0dc4cce..b719366d23 100644 --- a/packages/playwright-core/src/client/artifact.ts +++ b/packages/playwright-core/src/client/artifact.ts @@ -22,15 +22,13 @@ import { ChannelOwner } from './channelOwner'; import { Readable } from 'stream'; export class Artifact extends ChannelOwner { - _isRemote = false; - static from(channel: channels.ArtifactChannel): Artifact { return (channel as any)._object; } async pathAfterFinished(): Promise { - if (this._isRemote) - throw new Error(`Path is not available when using browserType.connect(). Use saveAs() to save a local copy.`); + if (this._connection.isRemote()) + throw new Error(`Path is not available when connecting remotely. Use saveAs() to save a local copy.`); return this._wrapApiCall(async (channel: channels.ArtifactChannel) => { return (await channel.pathAfterFinished()).value || null; }); @@ -38,7 +36,7 @@ export class Artifact extends ChannelOwner { return this._wrapApiCall(async (channel: channels.ArtifactChannel) => { - if (!this._isRemote) { + if (!this._connection.isRemote()) { await channel.saveAs({ path }); return; } diff --git a/packages/playwright-core/src/client/browser.ts b/packages/playwright-core/src/client/browser.ts index 47a087c1a6..9989ce0db4 100644 --- a/packages/playwright-core/src/client/browser.ts +++ b/packages/playwright-core/src/client/browser.ts @@ -29,7 +29,7 @@ export class Browser extends ChannelOwner(); private _isConnected = true; private _closedPromise: Promise; - _remoteType: 'owns-connection' | 'uses-connection' | null = null; + _shouldCloseConnectionOnClose = false; private _browserType!: BrowserType; readonly _name: string; @@ -109,7 +109,7 @@ export class Browser extends ChannelOwner { try { await this._wrapApiCall(async (channel: channels.BrowserChannel) => { - if (this._remoteType === 'owns-connection') + if (this._shouldCloseConnectionOnClose) this._connection.close(); else await channel.close(); diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index e6a44857de..341c9e4317 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -346,8 +346,6 @@ export class BrowserContext extends ChannelOwner pipe.close().catch(() => {}); const connection = new Connection(closePipe); + connection.markAsRemote(); const onPipeClosed = () => { // Emulate all pages, contexts and the browser closing upon disconnect. @@ -173,7 +174,7 @@ export class BrowserType extends ChannelOwner { playwright._cleanup(); @@ -221,7 +222,6 @@ export class BrowserType extends ChannelOwner void; + private _isRemote = false; constructor(onClose?: () => void) { super(); @@ -69,6 +70,14 @@ export class Connection extends EventEmitter { this._onClose = onClose; } + markAsRemote() { + this._isRemote = true; + } + + isRemote() { + return this._isRemote; + } + async initializePlaywright(): Promise { return await this._rootObject.initialize(); } diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index e4eb57a079..c4468643bc 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -128,7 +128,6 @@ export class Page extends ChannelOwner this.emit(Events.Page.DOMContentLoaded, this)); this._channel.on('download', ({ url, suggestedFilename, artifact }) => { const artifactObject = Artifact.from(artifact); - artifactObject._isRemote = !!this._browserContext._browser && !!this._browserContext._browser._remoteType; this.emit(Events.Page.Download, new Download(this, url, suggestedFilename, artifactObject)); }); this._channel.on('fileChooser', ({ element, isMultiple }) => this.emit(Events.Page.FileChooser, new FileChooser(this, ElementHandle.from(element), isMultiple))); @@ -250,7 +249,7 @@ export class Page extends ChannelOwner | null = null; private _artifactCallback = (artifact: Artifact) => {}; private _isRemote = false; - constructor(page: Page) { - const browser = page.context()._browser; - this._isRemote = !!browser && !!browser._remoteType; + constructor(page: Page, connection: Connection) { + this._isRemote = connection.isRemote(); this._artifact = Promise.race([ new Promise(f => this._artifactCallback = f), page._closedOrCrashedPromise.then(() => null), @@ -33,13 +33,12 @@ export class Video implements api.Video { } _artifactReady(artifact: Artifact) { - artifact._isRemote = this._isRemote; this._artifactCallback(artifact); } async path(): Promise { if (this._isRemote) - throw new Error(`Path is not available when using browserType.connect(). Use saveAs() to save a local copy.`); + throw new Error(`Path is not available when connecting remotely. Use saveAs() to save a local copy.`); const artifact = await this._artifact; if (!artifact) throw new Error('Page did not produce any video frames'); diff --git a/packages/playwright-core/src/remote/playwrightClient.ts b/packages/playwright-core/src/remote/playwrightClient.ts index bfa9b4cac6..29855d464e 100644 --- a/packages/playwright-core/src/remote/playwrightClient.ts +++ b/packages/playwright-core/src/remote/playwrightClient.ts @@ -32,6 +32,7 @@ export class PlaywrightClient { static async connect(options: PlaywrightClientConnectOptions): Promise { const { wsEndpoint, timeout = 30000 } = options; const connection = new Connection(); + connection.markAsRemote(); const ws = new WebSocket(wsEndpoint); const waitForNextTask = makeWaitForNextTask(); connection.onmessage = message => { diff --git a/tests/browsertype-connect.spec.ts b/tests/browsertype-connect.spec.ts index b11af6892f..dc56100d29 100644 --- a/tests/browsertype-connect.spec.ts +++ b/tests/browsertype-connect.spec.ts @@ -389,7 +389,7 @@ test('should saveAs videos from remote browser', async ({ browserType, startRemo await page.video().saveAs(savedAsPath); expect(fs.existsSync(savedAsPath)).toBeTruthy(); const error = await page.video().path().catch(e => e); - expect(error.message).toContain('Path is not available when using browserType.connect(). Use saveAs() to save a local copy.'); + expect(error.message).toContain('Path is not available when connecting remotely. Use saveAs() to save a local copy.'); }); test('should be able to connect 20 times to a single server without warnings', async ({ browserType, startRemoteServer }) => { @@ -428,7 +428,7 @@ test('should save download', async ({ server, browserType, startRemoteServer }, expect(fs.existsSync(nestedPath)).toBeTruthy(); expect(fs.readFileSync(nestedPath).toString()).toBe('Hello world'); const error = await download.path().catch(e => e); - expect(error.message).toContain('Path is not available when using browserType.connect(). Use saveAs() to save a local copy.'); + expect(error.message).toContain('Path is not available when connecting remotely. Use saveAs() to save a local copy.'); await browser.close(); }); diff --git a/tests/defaultbrowsercontext-1.spec.ts b/tests/defaultbrowsercontext-1.spec.ts index c3fc231d5c..d632a0a227 100644 --- a/tests/defaultbrowsercontext-1.spec.ts +++ b/tests/defaultbrowsercontext-1.spec.ts @@ -171,7 +171,9 @@ it('should support offline option', async ({ server, launchPersistent }) => { expect(error).toBeTruthy(); }); -it('should support acceptDownloads option', async ({ server, launchPersistent }) => { +it('should support acceptDownloads option', async ({ server, launchPersistent, mode }) => { + it.skip(mode === 'service', 'download.path() is not avaialble in remote mode'); + const { page } = await launchPersistent({ acceptDownloads: true }); server.setRoute('/download', (req, res) => { res.setHeader('Content-Type', 'application/octet-stream'); diff --git a/tests/download.spec.ts b/tests/download.spec.ts index b4272b4903..ffc803d51e 100644 --- a/tests/download.spec.ts +++ b/tests/download.spec.ts @@ -21,6 +21,8 @@ import crypto from 'crypto'; import type { Download } from 'playwright-core'; it.describe('download event', () => { + it.skip(({ mode }) => mode === 'service', 'download.path() is not available in remote mode'); + it.beforeEach(async ({ server }) => { server.setRoute('/download', (req, res) => { res.setHeader('Content-Type', 'application/octet-stream'); @@ -156,20 +158,6 @@ it.describe('download event', () => { await page.close(); }); - it('should save to user-specified path', async ({ browser, server }, testInfo) => { - const page = await browser.newPage({ acceptDownloads: true }); - await page.setContent(`download`); - const [ download ] = await Promise.all([ - page.waitForEvent('download'), - page.click('a') - ]); - const userPath = testInfo.outputPath('download.txt'); - await download.saveAs(userPath); - expect(fs.existsSync(userPath)).toBeTruthy(); - expect(fs.readFileSync(userPath).toString()).toBe('Hello world'); - await page.close(); - }); - it('should save to user-specified path without updating original path', async ({ browser, server }, testInfo) => { const page = await browser.newPage({ acceptDownloads: true }); await page.setContent(`download`); @@ -621,6 +609,30 @@ it('should be able to download a inline PDF file', async ({ browser, server, ass await page.close(); }); +it('should save to user-specified path', async ({ browser, server, mode }, testInfo) => { + server.setRoute('/download', (req, res) => { + res.setHeader('Content-Type', 'application/octet-stream'); + res.setHeader('Content-Disposition', 'attachment'); + res.end(`Hello world`); + }); + + const page = await browser.newPage({ acceptDownloads: true }); + await page.setContent(`download`); + const [ download ] = await Promise.all([ + page.waitForEvent('download'), + page.click('a') + ]); + if (mode === 'service') { + const error = await download.path().catch(e => e); + expect(error.message).toContain('Path is not available when connecting remotely. Use saveAs() to save a local copy.'); + } + const userPath = testInfo.outputPath('download.txt'); + await download.saveAs(userPath); + expect(fs.existsSync(userPath)).toBeTruthy(); + expect(fs.readFileSync(userPath).toString()).toBe('Hello world'); + await page.close(); +}); + async function assertDownloadToPDF(download: Download, filePath: string) { expect(download.suggestedFilename()).toBe(path.basename(filePath)); const stream = await download.createReadStream(); diff --git a/tests/downloads-path.spec.ts b/tests/downloads-path.spec.ts index 3dfaa2e4bb..7c31e38837 100644 --- a/tests/downloads-path.spec.ts +++ b/tests/downloads-path.spec.ts @@ -19,6 +19,8 @@ import fs from 'fs'; import path from 'path'; it.describe('downloads path', () => { + it.skip(({ mode }) => mode === 'service', 'download.path() is not available in remote mode'); + it.beforeEach(async ({ server }) => { server.setRoute('/download', (req, res) => { res.setHeader('Content-Type', 'application/octet-stream'); diff --git a/tests/video.spec.ts b/tests/video.spec.ts index 8e7896c035..bad15239a5 100644 --- a/tests/video.spec.ts +++ b/tests/video.spec.ts @@ -152,6 +152,7 @@ function expectRedFrames(videoFile: string, size: { width: number, height: numbe it.describe('screencast', () => { it.slow(); + it.skip(({ mode }) => mode === 'service', 'video.path() is not avaialble in remote mode'); it('videoSize should require videosPath', async ({ browser }) => { const error = await browser.newContext({ videoSize: { width: 100, height: 100 } }).catch(e => e); @@ -218,26 +219,6 @@ it.describe('screencast', () => { expect(fs.existsSync(path)).toBeTruthy(); }); - it('should saveAs video', async ({ browser }, testInfo) => { - const videosPath = testInfo.outputPath(''); - const size = { width: 320, height: 240 }; - const context = await browser.newContext({ - recordVideo: { - dir: videosPath, - size - }, - viewport: size, - }); - const page = await context.newPage(); - await page.evaluate(() => document.body.style.backgroundColor = 'red'); - await page.waitForTimeout(1000); - await context.close(); - - const saveAsPath = testInfo.outputPath('my-video.webm'); - await page.video().saveAs(saveAsPath); - expect(fs.existsSync(saveAsPath)).toBeTruthy(); - }); - it('saveAs should throw when no video frames', async ({ browser, browserName }, testInfo) => { const videosPath = testInfo.outputPath(''); const size = { width: 320, height: 240 }; @@ -668,5 +649,26 @@ it.describe('screencast', () => { const files = fs.readdirSync(videoDir); expect(files.length).toBe(1); }); - +}); + +it('should saveAs video', async ({ browser }, testInfo) => { + it.slow(); + + const videosPath = testInfo.outputPath(''); + const size = { width: 320, height: 240 }; + const context = await browser.newContext({ + recordVideo: { + dir: videosPath, + size + }, + viewport: size, + }); + const page = await context.newPage(); + await page.evaluate(() => document.body.style.backgroundColor = 'red'); + await page.waitForTimeout(1000); + await context.close(); + + const saveAsPath = testInfo.outputPath('my-video.webm'); + await page.video().saveAs(saveAsPath); + expect(fs.existsSync(saveAsPath)).toBeTruthy(); });