From 9aa9d6bc1d576a27ab3b9ee523208ddab6e05c54 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 8 Jun 2020 21:45:35 -0700 Subject: [PATCH] feat(downloads): accept downloads in persistent, allow specifying the downloadsPath (#2503) --- browsers.json | 2 +- docs/api.md | 4 + package-lock.json | 2 +- src/browserContext.ts | 13 +-- src/chromium/crBrowser.ts | 1 - src/helper.ts | 8 ++ src/server/browserType.ts | 33 +++++--- src/server/chromium.ts | 4 +- src/server/firefox.ts | 4 +- src/server/processLauncher.ts | 9 +- src/server/webkit.ts | 4 +- test/defaultbrowsercontext.spec.js | 11 --- test/downloadsPath.spec.js | 131 +++++++++++++++++++++++++++++ test/test.config.js | 1 + 14 files changed, 177 insertions(+), 50 deletions(-) create mode 100644 test/downloadsPath.spec.js diff --git a/browsers.json b/browsers.json index da0af913e4..76b513bbf0 100644 --- a/browsers.json +++ b/browsers.json @@ -6,7 +6,7 @@ }, { "name": "firefox", - "revision": "1103" + "revision": "1106" }, { "name": "webkit", diff --git a/docs/api.md b/docs/api.md index 3d7e39fc18..6a04e7df1c 100644 --- a/docs/api.md +++ b/docs/api.md @@ -4002,6 +4002,7 @@ This methods attaches Playwright to an existing browser instance. - `bypass` <[string]> Optional coma-separated domains to bypass proxy, for example `".com, chromium.org, .domain.com"`. - `username` <[string]> Optional username to use if HTTP proxy requires authentication. - `password` <[string]> Optional password to use if HTTP proxy requires authentication. + - `downloadsPath` <[string]> If specified, accepted downloads are downloaded into this folder. Otherwise, temporary folder is created and is deleted when browser is closed. - `firefoxUserPrefs` <[Object]> Firefox user preferences. Learn more about the Firefox user preferences at [`about:config`](https://support.mozilla.org/en-US/kb/about-config-editor-firefox). - `handleSIGINT` <[boolean]> Close the browser process on Ctrl-C. Defaults to `true`. - `handleSIGTERM` <[boolean]> Close the browser process on SIGTERM. Defaults to `true`. @@ -4041,6 +4042,8 @@ const browser = await chromium.launch({ // Or 'firefox' or 'webkit'. - `bypass` <[string]> Optional coma-separated domains to bypass proxy, for example `".com, chromium.org, .domain.com"`. - `username` <[string]> Optional username to use if HTTP proxy requires authentication. - `password` <[string]> Optional password to use if HTTP proxy requires authentication. + - `acceptDownloads` <[boolean]> Whether to automatically download all the attachments. Defaults to `false` where all the downloads are canceled. + - `downloadsPath` <[string]> If specified, accepted downloads are downloaded into this folder. Otherwise, temporary folder is created and is deleted when browser is closed. - `handleSIGINT` <[boolean]> Close the browser process on Ctrl-C. Defaults to `true`. - `handleSIGTERM` <[boolean]> Close the browser process on SIGTERM. Defaults to `true`. - `handleSIGHUP` <[boolean]> Close the browser process on SIGHUP. Defaults to `true`. @@ -4088,6 +4091,7 @@ Launches browser that uses persistent storage located at `userDataDir` and retur - `bypass` <[string]> Optional coma-separated domains to bypass proxy, for example `".com, chromium.org, .domain.com"`. - `username` <[string]> Optional username to use if HTTP proxy requires authentication. - `password` <[string]> Optional password to use if HTTP proxy requires authentication. + - `downloadsPath` <[string]> If specified, accepted downloads are downloaded into this folder. Otherwise, temporary folder is created and is deleted when browser is closed. - `firefoxUserPrefs` <[Object]> Firefox user preferences. Learn more about the Firefox user preferences at [`about:config`](https://support.mozilla.org/en-US/kb/about-config-editor-firefox). - `handleSIGINT` <[boolean]> Close the browser process on Ctrl-C. Defaults to `true`. - `handleSIGTERM` <[boolean]> Close the browser process on SIGTERM. Defaults to `true`. diff --git a/package-lock.json b/package-lock.json index b6b2c79387..2e656d7432 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "playwright-internal", - "version": "1.0.0-post", + "version": "1.1.0-post", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/src/browserContext.ts b/src/browserContext.ts index 3d35b3e139..9ea552630b 100644 --- a/src/browserContext.ts +++ b/src/browserContext.ts @@ -28,7 +28,7 @@ import { Log, InnerLogger, Logger, RootLogger } from './logger'; import { FunctionWithSource } from './frames'; import * as debugSupport from './debug/debugSupport'; -export type PersistentContextOptions = { +type CommonContextOptions = { viewport?: types.Size | null, ignoreHTTPSErrors?: boolean, javaScriptEnabled?: boolean, @@ -45,10 +45,11 @@ export type PersistentContextOptions = { isMobile?: boolean, hasTouch?: boolean, colorScheme?: types.ColorScheme, + acceptDownloads?: boolean, }; -export type BrowserContextOptions = PersistentContextOptions & { - acceptDownloads?: boolean, +export type PersistentContextOptions = CommonContextOptions; +export type BrowserContextOptions = CommonContextOptions & { logger?: Logger, }; @@ -278,12 +279,6 @@ export function validateBrowserContextOptions(options: BrowserContextOptions): B return result; } -export function validatePersistentContextOptions(options: PersistentContextOptions): PersistentContextOptions { - if ((options as any).acceptDownloads !== undefined) - throw new Error(`Option "acceptDownloads" is not supported for persistent context`); - return validateBrowserContextOptions(options); -} - export function verifyGeolocation(geolocation: types.Geolocation): types.Geolocation { const result = { ...geolocation }; result.accuracy = result.accuracy || 0; diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts index 8e23b711d2..b7174be1ba 100644 --- a/src/chromium/crBrowser.ts +++ b/src/chromium/crBrowser.ts @@ -54,7 +54,6 @@ export class CRBrowser extends BrowserBase { await session.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }); return browser; } - browser._defaultContext = new CRBrowserContext(browser, null, options.persistent); const existingTargetAttachPromises: Promise[] = []; diff --git a/src/helper.ts b/src/helper.ts index 72c597f6de..58ca016724 100644 --- a/src/helper.ts +++ b/src/helper.ts @@ -18,8 +18,10 @@ import * as crypto from 'crypto'; import { EventEmitter } from 'events'; import * as fs from 'fs'; +import * as removeFolder from 'rimraf'; import * as util from 'util'; import * as types from './types'; +const removeFolderAsync = util.promisify(removeFolder); export type RegisteredListener = { emitter: EventEmitter; @@ -270,6 +272,12 @@ class Helper { return { width, height }; return null; } + + static async removeFolders(dirs: string[]) { + await Promise.all(dirs.map(dir => { + return removeFolderAsync(dir).catch((err: Error) => console.error(err)); + })); + } } export function assert(value: any, message?: string): asserts value { diff --git a/src/server/browserType.ts b/src/server/browserType.ts index 7c18974b99..a9ba96f80b 100644 --- a/src/server/browserType.ts +++ b/src/server/browserType.ts @@ -18,7 +18,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import * as util from 'util'; -import { BrowserContext, PersistentContextOptions, validatePersistentContextOptions, verifyProxySettings } from '../browserContext'; +import { BrowserContext, PersistentContextOptions, verifyProxySettings, validateBrowserContextOptions } from '../browserContext'; import { BrowserServer, WebSocketWrapper } from './browserServer'; import * as browserPaths from '../install/browserPaths'; import { Logger, RootLogger, InnerLogger } from '../logger'; @@ -32,19 +32,13 @@ import { Progress, runAbortableTask } from '../progress'; import { ProxySettings } from '../types'; import { TimeoutSettings } from '../timeoutSettings'; -export type BrowserArgOptions = { - headless?: boolean, - args?: string[], - devtools?: boolean, - proxy?: ProxySettings, -}; - export type FirefoxUserPrefsOptions = { firefoxUserPrefs?: { [key: string]: string | number | boolean }, }; -type LaunchOptionsBase = BrowserArgOptions & { +export type LaunchOptionsBase = { executablePath?: string, + args?: string[], ignoreDefaultArgs?: boolean | string[], handleSIGINT?: boolean, handleSIGTERM?: boolean, @@ -52,6 +46,10 @@ type LaunchOptionsBase = BrowserArgOptions & { timeout?: number, logger?: Logger, env?: Env, + headless?: boolean, + devtools?: boolean, + proxy?: ProxySettings, + downloadsPath?: string, }; export function processBrowserArgOptions(options: LaunchOptionsBase): { devtools: boolean, headless: boolean } { @@ -77,6 +75,7 @@ export interface BrowserType { connect(options: ConnectOptions): Promise; } +const mkdirAsync = util.promisify(fs.mkdir); const mkdtempAsync = util.promisify(fs.mkdtemp); const DOWNLOADS_FOLDER = path.join(os.tmpdir(), 'playwright_downloads-'); @@ -116,7 +115,7 @@ export abstract class BrowserTypeBase implements BrowserType { async launchPersistentContext(userDataDir: string, options: LaunchOptions & PersistentContextOptions = {}): Promise { assert(!(options as any).port, 'Cannot specify a port without launching as a server.'); - const persistent = validatePersistentContextOptions(options); + const persistent = validateBrowserContextOptions(options); const logger = new RootLogger(options.logger); const browser = await runAbortableTask(progress => this._innerLaunch(progress, options, logger, persistent, userDataDir), logger, TimeoutSettings.timeout(options)); return browser._defaultContext!; @@ -179,8 +178,16 @@ export abstract class BrowserTypeBase implements BrowserType { handleSIGHUP = true, } = options; - const downloadsPath = await mkdtempAsync(DOWNLOADS_FOLDER); - const tempDirectories = [downloadsPath]; + const tempDirectories = []; + let downloadsPath: string; + if (options.downloadsPath) { + downloadsPath = options.downloadsPath; + await mkdirAsync(options.downloadsPath, { recursive: true }); + } else { + downloadsPath = await mkdtempAsync(DOWNLOADS_FOLDER); + tempDirectories.push(downloadsPath); + } + if (!userDataDir) { userDataDir = await mkdtempAsync(path.join(os.tmpdir(), `playwright_${this._name}dev_profile-`)); tempDirectories.push(userDataDir); @@ -239,7 +246,7 @@ export abstract class BrowserTypeBase implements BrowserType { return { browserServer, downloadsPath, transport }; } - abstract _defaultArgs(options: BrowserArgOptions, isPersistent: boolean, userDataDir: string): string[]; + abstract _defaultArgs(options: LaunchOptionsBase, isPersistent: boolean, userDataDir: string): string[]; abstract _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise; abstract _wrapTransportWithWebSocket(transport: ConnectionTransport, logger: InnerLogger, port: number): WebSocketWrapper; abstract _amendEnvironment(env: Env, userDataDir: string, executable: string, browserArguments: string[]): Env; diff --git a/src/server/chromium.ts b/src/server/chromium.ts index bef71ba4b1..4b4578cd29 100644 --- a/src/server/chromium.ts +++ b/src/server/chromium.ts @@ -21,7 +21,7 @@ import { CRBrowser } from '../chromium/crBrowser'; import * as ws from 'ws'; import { Env } from './processLauncher'; import { kBrowserCloseMessageId } from '../chromium/crConnection'; -import { BrowserArgOptions, BrowserTypeBase, processBrowserArgOptions } from './browserType'; +import { LaunchOptionsBase, BrowserTypeBase, processBrowserArgOptions } from './browserType'; import { WebSocketWrapper } from './browserServer'; import { ConnectionTransport, ProtocolRequest } from '../transport'; import { InnerLogger, logError } from '../logger'; @@ -77,7 +77,7 @@ export class Chromium extends BrowserTypeBase { return wrapTransportWithWebSocket(transport, logger, port); } - _defaultArgs(options: BrowserArgOptions, isPersistent: boolean, userDataDir: string): string[] { + _defaultArgs(options: LaunchOptionsBase, isPersistent: boolean, userDataDir: string): string[] { const { devtools, headless } = processBrowserArgOptions(options); const { args = [], proxy } = options; const userDataDirArg = args.find(arg => arg.startsWith('--user-data-dir')); diff --git a/src/server/firefox.ts b/src/server/firefox.ts index b0121749c1..91efcc03fc 100644 --- a/src/server/firefox.ts +++ b/src/server/firefox.ts @@ -23,7 +23,7 @@ import { FFBrowser } from '../firefox/ffBrowser'; import { kBrowserCloseMessageId } from '../firefox/ffConnection'; import { helper } from '../helper'; import { WebSocketWrapper } from './browserServer'; -import { BrowserArgOptions, BrowserTypeBase, processBrowserArgOptions, FirefoxUserPrefsOptions } from './browserType'; +import { LaunchOptionsBase, BrowserTypeBase, processBrowserArgOptions, FirefoxUserPrefsOptions } from './browserType'; import { Env } from './processLauncher'; import { ConnectionTransport, SequenceNumberMixer } from '../transport'; import { InnerLogger, logError } from '../logger'; @@ -57,7 +57,7 @@ export class Firefox extends BrowserTypeBase { return wrapTransportWithWebSocket(transport, logger, port); } - _defaultArgs(options: BrowserArgOptions & FirefoxUserPrefsOptions, isPersistent: boolean, userDataDir: string): string[] { + _defaultArgs(options: LaunchOptionsBase & FirefoxUserPrefsOptions, isPersistent: boolean, userDataDir: string): string[] { const { devtools, headless } = processBrowserArgOptions(options); const { args = [], proxy } = options; if (devtools) diff --git a/src/server/processLauncher.ts b/src/server/processLauncher.ts index 2e4ec4611b..e38b27a983 100644 --- a/src/server/processLauncher.ts +++ b/src/server/processLauncher.ts @@ -20,12 +20,9 @@ import { Log } from '../logger'; import * as readline from 'readline'; import * as removeFolder from 'rimraf'; import * as stream from 'stream'; -import * as util from 'util'; import { helper } from '../helper'; import { Progress } from '../progress'; -const removeFolderAsync = util.promisify(removeFolder); - export const browserLog: Log = { name: 'browser', }; @@ -67,11 +64,7 @@ type LaunchResult = { }; export async function launchProcess(options: LaunchProcessOptions): Promise { - const cleanup = async () => { - await Promise.all(options.tempDirectories.map(dir => { - return removeFolderAsync(dir).catch((err: Error) => console.error(err)); - })); - }; + const cleanup = () => helper.removeFolders(options.tempDirectories); const progress = options.progress; const stdio: ('ignore' | 'pipe')[] = options.pipe ? ['ignore', 'pipe', 'pipe', 'pipe', 'pipe'] : ['ignore', 'pipe', 'pipe']; diff --git a/src/server/webkit.ts b/src/server/webkit.ts index 683c2ca1d1..b9c3065bd5 100644 --- a/src/server/webkit.ts +++ b/src/server/webkit.ts @@ -20,7 +20,7 @@ import { Env } from './processLauncher'; import * as path from 'path'; import { helper } from '../helper'; import { kBrowserCloseMessageId } from '../webkit/wkConnection'; -import { BrowserArgOptions, BrowserTypeBase, processBrowserArgOptions } from './browserType'; +import { LaunchOptionsBase, BrowserTypeBase, processBrowserArgOptions } from './browserType'; import { ConnectionTransport, SequenceNumberMixer } from '../transport'; import * as ws from 'ws'; import { WebSocketWrapper } from './browserServer'; @@ -49,7 +49,7 @@ export class WebKit extends BrowserTypeBase { return wrapTransportWithWebSocket(transport, logger, port); } - _defaultArgs(options: BrowserArgOptions, isPersistent: boolean, userDataDir: string): string[] { + _defaultArgs(options: LaunchOptionsBase, isPersistent: boolean, userDataDir: string): string[] { const { devtools, headless } = processBrowserArgOptions(options); const { args = [], proxy } = options; if (devtools) diff --git a/test/defaultbrowsercontext.spec.js b/test/defaultbrowsercontext.spec.js index 2a68873d09..3b46ea47d7 100644 --- a/test/defaultbrowsercontext.spec.js +++ b/test/defaultbrowsercontext.spec.js @@ -374,15 +374,4 @@ describe('launchPersistentContext()', function() { expect(error).toBe(e); await removeUserDataDir(userDataDir); }); - it('should throw on unsupported options', async ({browserType, defaultBrowserOptions}) => { - const userDataDir = await makeUserDataDir(); - const optionNames = [ 'acceptDownloads' ]; - for (const option of optionNames) { - const options = { ...defaultBrowserOptions }; - options[option] = 'hello'; - const error = await browserType.launchPersistentContext(userDataDir, options).catch(e => e); - expect(error.message).toBe(`Option "${option}" is not supported for persistent context`); - } - await removeUserDataDir(userDataDir); - }); }); diff --git a/test/downloadsPath.spec.js b/test/downloadsPath.spec.js new file mode 100644 index 0000000000..cd47ceb82c --- /dev/null +++ b/test/downloadsPath.spec.js @@ -0,0 +1,131 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const path = require('path'); +const fs = require('fs'); +const util = require('util'); +const utils = require('./utils'); +const os = require('os'); +const removeFolder = require('rimraf'); +const mkdtempAsync = util.promisify(fs.mkdtemp); +const removeFolderAsync = util.promisify(removeFolder); + +const {FFOX, CHROMIUM, WEBKIT} = utils.testOptions(browserType); + +describe('browserType.launch({downloadsPath})', function() { + beforeEach(async(state) => { + state.downloadsPath = await mkdtempAsync(path.join(os.tmpdir(), 'playwright-test-')); + state.server.setRoute('/download', (req, res) => { + res.setHeader('Content-Type', 'application/octet-stream'); + res.setHeader('Content-Disposition', 'attachment; filename=file.txt'); + res.end(`Hello world`); + }); + state.browser = await state.browserType.launch({ + ...state.defaultBrowserOptions, + downloadsPath: state.downloadsPath, + }); + }); + afterEach(async(state) => { + await state.browser.close(); + await removeFolderAsync(state.downloadsPath); + }); + + it('should keep downloadsPath folder', async({browser, downloadsPath, server}) => { + const page = await browser.newPage(); + await page.setContent(`download`); + const [ download ] = await Promise.all([ + page.waitForEvent('download'), + page.click('a') + ]); + expect(download.url()).toBe(`${server.PREFIX}/download`); + expect(download.suggestedFilename()).toBe(`file.txt`); + await download.path().catch(e => error = e); + await page.close(); + await browser.close(); + expect(fs.existsSync(downloadsPath)).toBeTruthy(); + }); + it('should delete downloads when context closes', async({browser, downloadsPath, server}) => { + const page = await browser.newPage({ acceptDownloads: true }); + await page.setContent(`download`); + const [ download ] = await Promise.all([ + page.waitForEvent('download'), + page.click('a') + ]); + const path = await download.path(); + expect(fs.existsSync(path)).toBeTruthy(); + await page.close(); + expect(fs.existsSync(path)).toBeFalsy(); + }); + it('should report downloads in downloadsPath folder', async({browser, downloadsPath, server}) => { + const page = await browser.newPage({ acceptDownloads: true }); + await page.setContent(`download`); + const [ download ] = await Promise.all([ + page.waitForEvent('download'), + page.click('a') + ]); + const path = await download.path(); + expect(path.startsWith(downloadsPath)).toBeTruthy(); + await page.close(); + }); +}); + +describe('browserType.launchPersistent({acceptDownloads})', function() { + beforeEach(async(state) => { + state.userDataDir = await mkdtempAsync(path.join(os.tmpdir(), 'playwright-test-')); + state.downloadsPath = await mkdtempAsync(path.join(os.tmpdir(), 'playwright-test-')); + state.server.setRoute('/download', (req, res) => { + res.setHeader('Content-Type', 'application/octet-stream'); + res.setHeader('Content-Disposition', 'attachment; filename=file.txt'); + res.end(`Hello world`); + }); + state.context = await state.browserType.launchPersistentContext( + state.userDataDir, + { + ...state.defaultBrowserOptions, + downloadsPath: state.downloadsPath, + acceptDownloads: true + }); + state.page = state.context.pages()[0]; + state.page.setContent(`download`); + }); + afterEach(async(state) => { + await state.context.close(); + await removeFolderAsync(state.userDataDir); + await removeFolderAsync(state.downloadsPath); + }); + + it('should accept downloads', async({context, page, downloadsPath, server}) => { + const [ download ] = await Promise.all([ + page.waitForEvent('download'), + page.click('a') + ]); + expect(download.url()).toBe(`${server.PREFIX}/download`); + expect(download.suggestedFilename()).toBe(`file.txt`); + const path = await download.path(); + expect(path.startsWith(downloadsPath)).toBeTruthy(); + await context.close(); + }); + + it('should not delete downloads when the context closes', async({page, context}) => { + const [ download ] = await Promise.all([ + page.waitForEvent('download'), + page.click('a') + ]); + const path = await download.path(); + await context.close(); + expect(fs.existsSync(path)).toBeTruthy(); + }); +}); diff --git a/test/test.config.js b/test/test.config.js index 54a3a875c9..bde7c822a2 100644 --- a/test/test.config.js +++ b/test/test.config.js @@ -223,6 +223,7 @@ module.exports = { { files: [ './defaultbrowsercontext.spec.js', + './downloadsPath.spec.js', './fixtures.spec.js', './launcher.spec.js', './logger.spec.js',