feat(downloads): accept downloads in persistent, allow specifying the…

… downloadsPath (#2503)

browsers.json

@@ -6,7 +6,7 @@
     },
     {
       "name": "firefox",
-      "revision": "1103"
+      "revision": "1106"
     },
     {
       "name": "webkit",

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`.

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;

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<any>[] = [];

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 {

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,26 +32,24 @@ 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,
   handleSIGHUP?: boolean,
   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<Browser>;
 }
 
+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<BrowserContext> {
     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<BrowserBase>;
   abstract _wrapTransportWithWebSocket(transport: ConnectionTransport, logger: InnerLogger, port: number): WebSocketWrapper;
   abstract _amendEnvironment(env: Env, userDataDir: string, executable: string, browserArguments: string[]): Env;

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

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)

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<LaunchResult> {
-  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'];

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)

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