From 51645932b7dc3c3f3d958deb667c22b3519bcc91 Mon Sep 17 00:00:00 2001
From: Andrey Lushnikov <aslushnikov@gmail.com>
Date: Wed, 16 May 2018 18:09:59 -0700
Subject: [PATCH] refactor: do not pass user options to browser constructor
 (#2562)

This patch starts explicitly passing allowed options to the `Browser`
class. This, for example, makes it impossible to pass `appMode` as
an option to the `pptr.connect`.
---
 lib/Browser.js  | 18 ++++++++++--------
 lib/Launcher.js |  7 +++++--
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/lib/Browser.js b/lib/Browser.js
index a217c48f698..b56cc9440f6 100644
--- a/lib/Browser.js
+++ b/lib/Browser.js
@@ -23,14 +23,15 @@ class Browser extends EventEmitter {
   /**
    * @param {!Puppeteer.Connection} connection
    * @param {!Array<string>} contextIds
-   * @param {!BrowserOptions=} options
+   * @param {boolean} ignoreHTTPSErrors
+   * @param {boolean} setDefaultViewport
    * @param {?Puppeteer.ChildProcess} process
    * @param {(function():Promise)=} closeCallback
    */
-  constructor(connection, contextIds, options = {}, process, closeCallback) {
+  constructor(connection, contextIds, ignoreHTTPSErrors, setDefaultViewport, process, closeCallback) {
     super();
-    this._ignoreHTTPSErrors = !!options.ignoreHTTPSErrors;
-    this._appMode = !!options.appMode;
+    this._ignoreHTTPSErrors = ignoreHTTPSErrors;
+    this._setDefaultViewport = setDefaultViewport;
     this._process = process;
     this._screenshotTaskQueue = new TaskQueue();
     this._connection = connection;
@@ -87,12 +88,13 @@ class Browser extends EventEmitter {
   /**
    * @param {!Puppeteer.Connection} connection
    * @param {!Array<string>} contextIds
-   * @param {!BrowserOptions=} options
+   * @param {boolean} ignoreHTTPSErrors
+   * @param {boolean} appMode
    * @param {?Puppeteer.ChildProcess} process
    * @param {function()=} closeCallback
    */
-  static async create(connection, contextIds, options, process, closeCallback) {
-    const browser = new Browser(connection, contextIds, options, process, closeCallback);
+  static async create(connection, contextIds, ignoreHTTPSErrors, appMode, process, closeCallback) {
+    const browser = new Browser(connection, contextIds, ignoreHTTPSErrors, appMode, process, closeCallback);
     await connection.send('Target.setDiscoverTargets', {discover: true});
     return browser;
   }
@@ -105,7 +107,7 @@ class Browser extends EventEmitter {
     const {browserContextId} = targetInfo;
     const context = (browserContextId && this._contexts.has(browserContextId)) ? this._contexts.get(browserContextId) : this._defaultContext;
 
-    const target = new Target(targetInfo, context, () => this._connection.createSession(targetInfo.targetId), this._ignoreHTTPSErrors, !this._appMode, this._screenshotTaskQueue);
+    const target = new Target(targetInfo, context, () => this._connection.createSession(targetInfo.targetId), this._ignoreHTTPSErrors, this._setDefaultViewport, this._screenshotTaskQueue);
     console.assert(!this._targets.has(event.targetInfo.targetId), 'Target should not exist before targetCreated');
     this._targets.set(event.targetInfo.targetId, target);
 
diff --git a/lib/Launcher.js b/lib/Launcher.js
index a8cc8ef1513..b3866dd4543 100644
--- a/lib/Launcher.js
+++ b/lib/Launcher.js
@@ -159,7 +159,9 @@ class Launcher {
       } else {
         connection = Connection.createForPipe(/** @type {!NodeJS.WritableStream} */(chromeProcess.stdio[3]), /** @type {!NodeJS.ReadableStream} */ (chromeProcess.stdio[4]), connectionDelay);
       }
-      return Browser.create(connection, [], options, chromeProcess, gracefullyCloseChrome);
+      const ignoreHTTPSErrors = !!options.ignoreHTTPSErrors;
+      const setDefaultViewport = !options.appMode;
+      return Browser.create(connection, [], ignoreHTTPSErrors, setDefaultViewport, chromeProcess, gracefullyCloseChrome);
     } catch (e) {
       killChrome();
       throw e;
@@ -227,7 +229,8 @@ class Launcher {
     const connectionDelay = options.slowMo || 0;
     const connection = await Connection.createForWebSocket(options.browserWSEndpoint, connectionDelay);
     const {browserContextIds} = await connection.send('Target.getBrowserContexts');
-    return Browser.create(connection, browserContextIds, options, null, () => connection.send('Browser.close').catch(debugError));
+    const ignoreHTTPSErrors = !!options.ignoreHTTPSErrors;
+    return Browser.create(connection, browserContextIds, ignoreHTTPSErrors, true /* setDefaultViewport */, null, () => connection.send('Browser.close').catch(debugError));
   }
 }