refactor(firefox): migrate onto Juggler flatten protocol (#4033)

Juggler now implements the same "flatten" protocol as CDP.
This patch:

* copies `Connection.js` from original Puppeteer (with a few renames, e.g. `CDPSesssion` -> `JugglerSession`).
* migrates code to support protocol-level sessions

experimental/puppeteer-firefox/lib/Browser.js

@@ -11,9 +11,9 @@ class Browser extends EventEmitter {
    * @param {function():void} closeCallback
    */
   static async create(connection, defaultViewport, process, closeCallback) {
-    const {browserContextIds} = await connection.send('Browser.getBrowserContexts');
+    const {browserContextIds} = await connection.send('Target.getBrowserContexts');
     const browser = new Browser(connection, browserContextIds, defaultViewport, process, closeCallback);
-    await connection.send('Browser.enable');
+    await connection.send('Target.enable');
     return browser;
   }
 
@@ -43,9 +43,9 @@ class Browser extends EventEmitter {
     this._connection.on(Events.Connection.Disconnected, () => this.emit(Events.Browser.Disconnected));
 
     this._eventListeners = [
-      helper.addEventListener(this._connection, 'Browser.targetCreated', this._onTargetCreated.bind(this)),
+      helper.addEventListener(this._connection, 'Target.targetCreated', this._onTargetCreated.bind(this)),
-      helper.addEventListener(this._connection, 'Browser.targetDestroyed', this._onTargetDestroyed.bind(this)),
+      helper.addEventListener(this._connection, 'Target.targetDestroyed', this._onTargetDestroyed.bind(this)),
-      helper.addEventListener(this._connection, 'Browser.targetInfoChanged', this._onTargetInfoChanged.bind(this)),
+      helper.addEventListener(this._connection, 'Target.targetInfoChanged', this._onTargetInfoChanged.bind(this)),
     ];
   }
 
@@ -61,7 +61,7 @@ class Browser extends EventEmitter {
    * @return {!BrowserContext}
    */
   async createIncognitoBrowserContext() {
-    const {browserContextId} = await this._connection.send('Browser.createBrowserContext');
+    const {browserContextId} = await this._connection.send('Target.createBrowserContext');
     const context = new BrowserContext(this._connection, this, browserContextId);
     this._contexts.set(browserContextId, context);
     return context;
@@ -79,7 +79,7 @@ class Browser extends EventEmitter {
   }
 
   async _disposeContext(browserContextId) {
-    await this._connection.send('Browser.removeBrowserContext', {browserContextId});
+    await this._connection.send('Target.removeBrowserContext', {browserContextId});
     this._contexts.delete(browserContextId);
   }
 
@@ -152,7 +152,7 @@ class Browser extends EventEmitter {
    * @return {Promise<Page>}
    */
   async _createPageInContext(browserContextId) {
-    const {targetId} = await this._connection.send('Browser.newPage', {
+    const {targetId} = await this._connection.send('Target.newPage', {
       browserContextId: browserContextId || undefined
     });
     const target = this._targets.get(targetId);
@@ -190,6 +190,7 @@ class Browser extends EventEmitter {
   _onTargetDestroyed({targetId}) {
     const target = this._targets.get(targetId);
     this._targets.delete(targetId);
+    target._closedCallback();
     this.emit(Events.Browser.TargetDestroyed, target);
     target.browserContext().emit(Events.BrowserContext.TargetDestroyed, target);
   }
@@ -228,6 +229,7 @@ class Target {
     this._pagePromise = null;
     this._url = url;
     this._openerId = openerId;
+    this._isClosedPromise = new Promise(fulfill => this._closedCallback = fulfill);
   }
 
   /**
@@ -256,8 +258,10 @@ class Target {
   }
 
   async page() {
-    if (this._type === 'page' && !this._pagePromise)
+    if (this._type === 'page' && !this._pagePromise) {
-      this._pagePromise = Page.create(this._connection, this, this._targetId, this._browser._defaultViewport);
+      const session = await this._connection.createSession(this._targetId);
+      this._pagePromise = Page.create(session, this, this._browser._defaultViewport);
+    }
     return this._pagePromise;
   }
 

experimental/puppeteer-firefox/lib/Connection.js

@@ -13,15 +13,14 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-const debugProtocol = require('debug')('hdfox:protocol');
+const {assert} = require('./helper');
-const EventEmitter = require('events');
 const {Events} = require('./Events');
+const debugProtocol = require('debug')('puppeteer:protocol');
+const EventEmitter = require('events');
 
-/**
- * @internal
- */
 class Connection extends EventEmitter {
   /**
+   * @param {string} url
    * @param {!Puppeteer.ConnectionTransport} transport
    * @param {number=} delay
    */
@@ -36,9 +35,30 @@ class Connection extends EventEmitter {
     this._transport = transport;
     this._transport.onmessage = this._onMessage.bind(this);
     this._transport.onclose = this._onClose.bind(this);
+    /** @type {!Map<string, !JugglerSession>}*/
+    this._sessions = new Map();
     this._closed = false;
   }
 
+  /**
+   * @param {!JugglerSession} session
+   * @return {!Connection}
+   */
+  static fromSession(session) {
+    return session._connection;
+  }
+
+  /**
+   * @param {string} sessionId
+   * @return {?JugglerSession}
+   */
+  session(sessionId) {
+    return this._sessions.get(sessionId) || null;
+  }
+
+  /**
+   * @return {string}
+   */
   url() {
     return this._url;
   }
@@ -49,15 +69,24 @@ class Connection extends EventEmitter {
    * @return {!Promise<?Object>}
    */
   send(method, params = {}) {
-    const id = ++this._lastId;
+    const id = this._rawSend({method, params});
-    const message = JSON.stringify({id, method, params});
-    debugProtocol('SEND ► ' + message);
-    this._transport.send(message);
     return new Promise((resolve, reject) => {
       this._callbacks.set(id, {resolve, reject, error: new Error(), method});
     });
   }
 
+  /**
+   * @param {*} message
+   * @return {number}
+   */
+  _rawSend(message) {
+    const id = ++this._lastId;
+    message = JSON.stringify(Object.assign({}, message, {id}));
+    debugProtocol('SEND ► ' + message);
+    this._transport.send(message);
+    return id;
+  }
+
   /**
    * @param {string} message
    */
@@ -66,7 +95,22 @@ class Connection extends EventEmitter {
       await new Promise(f => setTimeout(f, this._delay));
     debugProtocol('◀ RECV ' + message);
     const object = JSON.parse(message);
-    if (object.id) {
+    if (object.method === 'Target.attachedToTarget') {
+      const sessionId = object.params.sessionId;
+      const session = new JugglerSession(this, object.params.targetInfo.type, sessionId);
+      this._sessions.set(sessionId, session);
+    } else if (object.method === 'Browser.detachedFromTarget') {
+      const session = this._sessions.get(object.params.sessionId);
+      if (session) {
+        session._onClosed();
+        this._sessions.delete(object.params.sessionId);
+      }
+    }
+    if (object.sessionId) {
+      const session = this._sessions.get(object.sessionId);
+      if (session)
+        session._onMessage(object);
+    } else if (object.id) {
       const callback = this._callbacks.get(object.id);
       // Callbacks could be all rejected if someone has called `.dispose()`.
       if (callback) {
@@ -90,6 +134,9 @@ class Connection extends EventEmitter {
     for (const callback of this._callbacks.values())
       callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): Target closed.`));
     this._callbacks.clear();
+    for (const session of this._sessions.values())
+      session._onClosed();
+    this._sessions.clear();
     this.emit(Events.Connection.Disconnected);
   }
 
@@ -97,6 +144,76 @@ class Connection extends EventEmitter {
     this._onClose();
     this._transport.close();
   }
+
+  /**
+   * @param {string} targetId
+   * @return {!Promise<!JugglerSession>}
+   */
+  async createSession(targetId) {
+    const {sessionId} = await this.send('Target.attachToTarget', {targetId});
+    return this._sessions.get(sessionId);
+  }
+}
+
+class JugglerSession extends EventEmitter {
+  /**
+   * @param {!Connection} connection
+   * @param {string} targetType
+   * @param {string} sessionId
+   */
+  constructor(connection, targetType, sessionId) {
+    super();
+    /** @type {!Map<number, {resolve: function, reject: function, error: !Error, method: string}>}*/
+    this._callbacks = new Map();
+    this._connection = connection;
+    this._targetType = targetType;
+    this._sessionId = sessionId;
+  }
+
+  /**
+   * @param {string} method
+   * @param {!Object=} params
+   * @return {!Promise<?Object>}
+   */
+  send(method, params = {}) {
+    if (!this._connection)
+      return Promise.reject(new Error(`Protocol error (${method}): Session closed. Most likely the ${this._targetType} has been closed.`));
+    const id = this._connection._rawSend({sessionId: this._sessionId, method, params});
+    return new Promise((resolve, reject) => {
+      this._callbacks.set(id, {resolve, reject, error: new Error(), method});
+    });
+  }
+
+  /**
+   * @param {{id?: number, method: string, params: Object, error: {message: string, data: any}, result?: *}} object
+   */
+  _onMessage(object) {
+    if (object.id && this._callbacks.has(object.id)) {
+      const callback = this._callbacks.get(object.id);
+      this._callbacks.delete(object.id);
+      if (object.error)
+        callback.reject(createProtocolError(callback.error, callback.method, object));
+      else
+        callback.resolve(object.result);
+    } else {
+      assert(!object.id);
+      this.emit(object.method, object.params);
+    }
+  }
+
+  async detach() {
+    if (!this._connection)
+      throw new Error(`Session already detached. Most likely the ${this._targetType} has been closed.`);
+    await this._connection.send('Target.detachFromTarget',  {sessionId: this._sessionId});
+  }
+
+  _onClosed() {
+    for (const callback of this._callbacks.values())
+      callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): Target closed.`));
+    this._callbacks.clear();
+    this._connection = null;
+    this.emit(Events.JugglerSession.Disconnected);
+  }
 }
 
 /**
@@ -122,4 +239,4 @@ function rewriteError(error, message) {
   return error;
 }
 
-module.exports = {Connection};
+module.exports = {Connection, JugglerSession};

experimental/puppeteer-firefox/lib/Events.js

@@ -31,6 +31,10 @@ const Events = {
     Disconnected: Symbol('Events.Connection.Disconnected'),
   },
 
+  JugglerSession: {
+    Disconnected: Symbol('Events.JugglerSession.Disconnected'),
+  },
+
   FrameManager: {
     Load: Symbol('Events.FrameManager.Load'),
     DOMContentLoaded: Symbol('Events.FrameManager.DOMContentLoaded'),

experimental/puppeteer-firefox/lib/Page.js

@@ -15,49 +15,20 @@ const {NavigationWatchdog, NextNavigationWatchdog} = require('./NavigationWatchd
 
 const writeFileAsync = util.promisify(fs.writeFile);
 
-/**
- * @internal
- */
-class PageSession extends EventEmitter {
-  constructor(connection, targetId) {
-    super();
-    this._connection = connection;
-    this._targetId = targetId;
-    const wrapperSymbol = Symbol('listenerWrapper');
-
-    function wrapperListener(listener, params) {
-      if (params.targetId === targetId)
-        listener.call(null, params);
-    }
-
-    this.on('removeListener', (eventName, listener) => {
-      this._connection.removeListener(eventName, listener[wrapperSymbol]);
-    });
-    this.on('newListener', (eventName, listener) => {
-      if (!listener[wrapperSymbol])
-        listener[wrapperSymbol] = wrapperListener.bind(null, listener);
-      this._connection.on(eventName, listener[wrapperSymbol]);
-    });
-  }
-
-  async send(method, params = {}) {
-    params = Object.assign({}, params, {targetId: this._targetId});
-    return await this._connection.send(method, params);
-  }
-}
-
 class Page extends EventEmitter {
   /**
    *
-   * @param {!Puppeteer.Connection} connection
+   * @param {!Puppeteer.JugglerSession} connection
    * @param {!Puppeteer.Target} target
-   * @param {string} targetId
    * @param {?Puppeteer.Viewport} defaultViewport
    */
-  static async create(connection, target, targetId, defaultViewport) {
+  static async create(session, target, defaultViewport) {
-    const session = new PageSession(connection, targetId);
     const page = new Page(session, target);
-    await session.send('Page.enable');
+    await Promise.all([
+      session.send('Page.enable'),
+      session.send('Network.enable'),
+    ]);
+
     if (defaultViewport)
       await page.setViewport(defaultViewport);
     return page;
@@ -74,7 +45,7 @@ class Page extends EventEmitter {
     this._target = target;
     this._keyboard = new Keyboard(session);
     this._mouse = new Mouse(session, this._keyboard);
-    this._isClosed = false;
+    this._closed = false;
     this._networkManager = new NetworkManager(session);
     this._frameManager = new FrameManager(session, this, this._networkManager, this._timeoutSettings);
     this._networkManager.setFrameManager(this._frameManager);
@@ -82,7 +53,6 @@ class Page extends EventEmitter {
       helper.addEventListener(this._session, 'Page.uncaughtError', this._onUncaughtError.bind(this)),
       helper.addEventListener(this._session, 'Page.console', this._onConsole.bind(this)),
       helper.addEventListener(this._session, 'Page.dialogOpened', this._onDialogOpened.bind(this)),
-      helper.addEventListener(this._session, 'Browser.targetDestroyed', this._onClosed.bind(this)),
       helper.addEventListener(this._frameManager, Events.FrameManager.Load, () => this.emit(Events.Page.Load)),
       helper.addEventListener(this._frameManager, Events.FrameManager.DOMContentLoaded, () => this.emit(Events.Page.DOMContentLoaded)),
       helper.addEventListener(this._frameManager, Events.FrameManager.FrameAttached, frame => this.emit(Events.Page.FrameAttached, frame)),
@@ -94,6 +64,13 @@ class Page extends EventEmitter {
       helper.addEventListener(this._networkManager, Events.NetworkManager.RequestFailed, request => this.emit(Events.Page.RequestFailed, request)),
     ];
     this._viewport = null;
+    this._target._isClosedPromise.then(() => {
+      this._closed = true;
+      this._frameManager.dispose();
+      this._networkManager.dispose();
+      helper.removeEventListeners(this._eventListeners);
+      this.emit(Events.Page.Close);
+    });
   }
 
   /**
@@ -553,7 +530,9 @@ class Page extends EventEmitter {
     const {
       runBeforeUnload = false,
     } = options;
-    await this._session.send('Browser.closePage', { runBeforeUnload });
+    await this._session.send('Page.close', { runBeforeUnload });
+    if (!runBeforeUnload)
+      await this._target._isClosedPromise;
   }
 
   async content() {
@@ -568,11 +547,6 @@ class Page extends EventEmitter {
   }
 
   _onClosed() {
-    this._isClosed = true;
-    this._frameManager.dispose();
-    this._networkManager.dispose();
-    helper.removeEventListeners(this._eventListeners);
-    this.emit(Events.Page.Close);
   }
 
   _onConsole({type, args, frameId, location}) {
@@ -584,7 +558,7 @@ class Page extends EventEmitter {
    * @return {boolean}
    */
   isClosed() {
-    return this._isClosed;
+    return this._closed;
   }
 }
 

experimental/puppeteer-firefox/package.json

@@ -9,7 +9,7 @@
     "node": ">=8.9.4"
   },
   "puppeteer": {
-    "firefox_revision": "2ede4ae19f39ec7a1b73162a6004235908260dfe"
+    "firefox_revision": "387ac6bbbe5357d174e9fb3aa9b6f935113c315d"
   },
   "scripts": {
     "install": "node install.js",

test/navigation.spec.js

@@ -265,7 +265,7 @@ module.exports.addTests = function({testRunner, expect, Errors, CHROME}) {
       process.removeListener('warning', warningHandler);
       expect(warning).toBe(null);
     });
-    it_fails_ffox('should not leak listeners during navigation of 11 pages', async({page, context, server}) => {
+    it('should not leak listeners during navigation of 11 pages', async({page, context, server}) => {
       let warning = null;
       const warningHandler = w => warning = w;
       process.on('warning', warningHandler);

test/page.spec.js

@@ -36,10 +36,11 @@ module.exports.addTests = function({testRunner, expect, headless, Errors, Device
   describe('Page.close', function() {
     it('should reject all promises when page is closed', async({context}) => {
       const newPage = await context.newPage();
-      const neverResolves = newPage.evaluate(() => new Promise(r => {}));
-      newPage.close();
       let error = null;
-      await neverResolves.catch(e => error = e);
+      await Promise.all([
+        newPage.evaluate(() => new Promise(r => {})).catch(e => error = e),
+        newPage.close(),
+      ]);
       expect(error.message).toContain('Protocol error');
     });
     it('should not be visible in browser.pages', async({browser}) => {