From e2e43bc23d2a81eb9c9214fb64f5fff3b95e6590 Mon Sep 17 00:00:00 2001
From: Andrey Lushnikov <aslushnikov@gmail.com>
Date: Tue, 20 Nov 2018 14:21:13 -0800
Subject: [PATCH] fix(page): navigating 11 pages simultaneously should not
 throw warning (#3560)

NavigatorWatcher subscribes to Connection to get a `Disconnected` event,
causing us to hit the default max of 10 listeners constraint.

Technically we don't leak anything here and can safely bump
the maxListenersCount to Infinity.

However, we conveniently have `CDPSession`, and
can re-dispatch the event on it and keep the safety check in place.
---
 lib/Connection.js   |  6 ++++++
 lib/FrameManager.js |  4 ++--
 test/page.spec.js   | 12 ++++++++++++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/Connection.js b/lib/Connection.js
index 2d648cc8388..8127ef7da70 100644
--- a/lib/Connection.js
+++ b/lib/Connection.js
@@ -228,6 +228,7 @@ class CDPSession extends EventEmitter {
       callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): Target closed.`));
     this._callbacks.clear();
     this._connection = null;
+    this.emit(CDPSession.Events.Disconnected);
   }
 
   /**
@@ -240,6 +241,11 @@ class CDPSession extends EventEmitter {
     return session;
   }
 }
+
+CDPSession.Events = {
+  Disconnected: Symbol('CDPSession.Events.Disconnected'),
+};
+
 helper.tracePublicAPI(CDPSession);
 
 /**
diff --git a/lib/FrameManager.js b/lib/FrameManager.js
index b2fd414907c..2f6ca11f8e4 100644
--- a/lib/FrameManager.js
+++ b/lib/FrameManager.js
@@ -20,7 +20,7 @@ const {helper, assert} = require('./helper');
 const {ExecutionContext} = require('./ExecutionContext');
 const {TimeoutError} = require('./Errors');
 const {NetworkManager} = require('./NetworkManager');
-const {Connection} = require('./Connection');
+const {CDPSession} = require('./Connection');
 
 const readFileAsync = helper.promisify(fs.readFile);
 
@@ -1162,7 +1162,7 @@ class NavigatorWatcher {
     this._navigationRequest = null;
     this._hasSameDocumentNavigation = false;
     this._eventListeners = [
-      helper.addEventListener(Connection.fromSession(client), Connection.Events.Disconnected, () => this._terminate(new Error('Navigation failed because browser has disconnected!'))),
+      helper.addEventListener(client, CDPSession.Events.Disconnected, () => this._terminate(new Error('Navigation failed because browser has disconnected!'))),
       helper.addEventListener(this._frameManager, FrameManager.Events.LifecycleEvent, this._checkLifecycleComplete.bind(this)),
       helper.addEventListener(this._frameManager, FrameManager.Events.FrameNavigatedWithinDocument, this._navigatedWithinDocument.bind(this)),
       helper.addEventListener(this._frameManager, FrameManager.Events.FrameDetached, this._onFrameDetached.bind(this)),
diff --git a/test/page.spec.js b/test/page.spec.js
index a873194f065..344da91d61f 100644
--- a/test/page.spec.js
+++ b/test/page.spec.js
@@ -752,6 +752,18 @@ module.exports.addTests = function({testRunner, expect, headless}) {
       process.removeListener('warning', warningHandler);
       expect(warning).toBe(null);
     });
+    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);
+      await Promise.all([...Array(20)].map(async() => {
+        const page = await context.newPage();
+        await page.goto(server.EMPTY_PAGE);
+        await page.close();
+      }));
+      process.removeListener('warning', warningHandler);
+      expect(warning).toBe(null);
+    });
     it('should navigate to dataURL and fire dataURL requests', async({page, server}) => {
       const requests = [];
       page.on('request', request => requests.push(request));