unlok · Jun 23, 2020 · Jun 3, 2020 · DudaGod · Jun 17, 2020 · unlok
diff --git a/lib/browser-pool/basic-pool.js b/lib/browser-pool/basic-pool.js
@@ -20,11 +20,12 @@ module.exports = class BasicPool extends Pool {
         this._activeSessions = {};
     }
 
-    getBrowser(id) {
-        const browser = this._browserMgr.create(id);
+    getBrowser(id, opts = {}) {
+        const {version} = opts;
+        const browser = this._browserMgr.create(id, version);
 
         return this._browserMgr.start(browser)
-            .then(() => this.log(`browser ${browser.id} started`))
+            .then(() => this.log(`browser ${browser.fullId} started`))
             .then(() => this._browserMgr.onStart(browser))
             .then(() => {
                 if (this._cancelled) {
@@ -40,7 +41,8 @@ module.exports = class BasicPool extends Pool {
 
     freeBrowser(browser) {
         delete this._activeSessions[browser.sessionId];
-        this.log(`stop browser ${browser.id}`);
+
+        this.log(`stop browser ${browser.fullId}`);
 
         return this._browserMgr.onQuit(browser)
             .catch((err) => console.warn(err && err.stack || err))

diff --git a/lib/browser-pool/caching-pool.js b/lib/browser-pool/caching-pool.js
@@ -4,6 +4,7 @@ const Promise = require('bluebird');
 const Pool = require('./pool');
 const LimitedUseSet = require('./limited-use-set');
 const debug = require('debug');
+const {buildCompositeBrowserId} = require('./utils');
 
 module.exports = class CachingPool extends Pool {
     /**
@@ -15,35 +16,38 @@ module.exports = class CachingPool extends Pool {
         super();
 
         this.log = debug(`${opts.logNamespace}:pool:caching`);
-
         this.underlyingPool = underlyingPool;
         this._caches = {};
+        this._config = config;
+        this._logNamespace = opts.logNamespace;
+    }
 
-        const freeBrowser = underlyingPool.freeBrowser.bind(underlyingPool);
+    _getCacheFor(id, version) {
+        const compositeId = buildCompositeBrowserId(id, version);
 
-        config.getBrowserIds().forEach((id) => {
-            const browserConfig = config.forBrowser(id);
-            this._caches[id] = new LimitedUseSet({
-                // browser does not get put in a set on first usages, so if
-                // we want to limit it usage to N times, we must set N-1 limit
-                // for the set.
-                useLimit: browserConfig.sessionUseLimit - 1,
+        this.log(`request for ${compositeId}`);
 
-                finalize: freeBrowser,
-                logNamespace: opts.logNamespace
-            });
-        });
+        if (!this._caches[compositeId]) {
+            this.log(`init for ${compositeId}`);
+            this._initPool(id, version);
+        }
+
+        return this._caches[compositeId];
     }
 
-    getBrowser(id) {
-        this.log(`request for ${id}`);
-        const browser = this._caches[id].pop();
+    getBrowser(id, opts = {}) {
+        const {version} = opts;
+        const cache = this._getCacheFor(id, version);
+        const browser = cache.pop();
+
         if (!browser) {
-            this.log('no cached browser, requesting new');
-            return this.underlyingPool.getBrowser(id);
+            this.log(`no cached browser for ${buildCompositeBrowserId(id, version)}, requesting new`);
+
+            return this.underlyingPool.getBrowser(id, opts);
         }
 
-        this.log(`has cached browser ${browser}`);
+        this.log(`has cached browser ${browser.fullId}`);
+
         return browser.reset()
             .catch((e) => {
                 const reject = Promise.reject.bind(null, e);
@@ -53,6 +57,22 @@ module.exports = class CachingPool extends Pool {
             .then(() => browser);
     }
 
+    _initPool(browserId, version) {
+        const compositeId = buildCompositeBrowserId(browserId, version);
+        const freeBrowser = this.underlyingPool.freeBrowser.bind(this.underlyingPool);
+        const browserConfig = this._config.forBrowser(browserId);
+
+        this._caches[compositeId] = new LimitedUseSet({
+            formatItem: (item) => item.fullId,
+            // browser does not get put in a set on first usages, so if
+            // we want to limit it usage to N times, we must set N-1 limit
+            // for the set.
+            useLimit: browserConfig.sessionUseLimit - 1,
+            finalize: freeBrowser,
+            logNamespace: this._logNamespace
+        });
+    }
+
     /**
      * Free browser
      * @param {Browser} browser session instance
@@ -61,12 +81,30 @@ module.exports = class CachingPool extends Pool {
      * not be cached
      * @returns {Promise<undefined>}
      */
-    freeBrowser(browser, options) {
-        this.log(`free ${browser.id}`);
-        const shouldBeFreed = options && options.force;
-        return shouldBeFreed
-            ? this.underlyingPool.freeBrowser(browser)
-            : this._caches[browser.id].push(browser);
+    freeBrowser(browser, options = {}) {
+        const shouldFreeForNextRequest = () => {
+            const {compositeIdForNextRequest} = options;
+
+            if (!compositeIdForNextRequest) {
+                return false;
+            }
+
+            const {hasFreeSlots} = options;
+            const hasCacheForNextRequest = this._caches[options.compositeIdForNextRequest];
+
+            return !hasFreeSlots && !hasCacheForNextRequest;
+        };
+        const force = options.force || shouldFreeForNextRequest();
+
+        this.log(`free ${browser.fullId} force=${force}`);
+
+        if (force) {
+            return this.underlyingPool.freeBrowser(browser);
+        }
+
+        const cache = this._getCacheFor(browser.id, browser.version);
+
+        return cache.push(browser);
     }
 
     cancel() {

diff --git a/lib/browser-pool/limited-pool.js b/lib/browser-pool/limited-pool.js
@@ -5,6 +5,7 @@ const yallist = require('yallist');
 const Pool = require('./pool');
 const CancelledError = require('../errors/cancelled-error');
 const debug = require('debug');
+const {buildCompositeBrowserId} = require('./utils');
 
 module.exports = class LimitedPool extends Pool {
     static create(underlyingPool, opts) {
@@ -30,7 +31,8 @@ module.exports = class LimitedPool extends Pool {
     }
 
     getBrowser(id, opts = {}) {
-        this.log(`get browser ${id} (launched ${this._launched}, limit ${this._limit})`);
+        const optsToPrint = JSON.stringify(opts);
+        this.log(`get browser ${id} with opts:${optsToPrint} (launched ${this._launched}, limit ${this._limit})`);
 
         ++this._requests;
         return this._getBrowser(id, opts)
@@ -40,12 +42,20 @@ module.exports = class LimitedPool extends Pool {
             });
     }
 
-    freeBrowser(browser, opts) {
-        this.log(`free browser ${browser.id}`);
+    freeBrowser(browser, opts = {}) {
         --this._requests;
 
-        const force = opts && opts.force || this._launched > this._requests;
-        return this.underlyingPool.freeBrowser(browser, {force})
+        const nextRequest = this._lookAtNextRequest();
+        const compositeIdForNextRequest = nextRequest && buildCompositeBrowserId(nextRequest.id, nextRequest.version);
+        const hasFreeSlots = this._launched < this._limit;
+        const shouldFreeUnusedResource = this._launched > this._requests;
+        const force = opts.force || shouldFreeUnusedResource;
+        const optsForFree = {force, compositeIdForNextRequest, hasFreeSlots};
+
+        this.log(`free browser ${browser.fullId} with opts:${JSON.stringify(optsForFree)}`);
+
+        return this.underlyingPool
+            .freeBrowser(browser, optsForFree)
             .finally(() => this._launchNextBrowser());
     }
 
@@ -70,9 +80,12 @@ module.exports = class LimitedPool extends Pool {
         }
 
         this.log('queuing the request');
+
         const queue = opts.highPriority ? this._highPriorityRequestQueue : this._requestQueue;
+        const {version} = opts;
+
         return new Promise((resolve, reject) => {
-            queue.push({id, resolve, reject});
+            queue.push({id, version, resolve, reject});
         });
     }
 
@@ -81,20 +94,28 @@ module.exports = class LimitedPool extends Pool {
      * @returns {Promise<Browser>}
      */
     _newBrowser(id, opts) {
-        this.log('launching new browser', id);
+        this.log(`launching new browser ${id} with opts:${JSON.stringify(opts)}`);
+
         return this.underlyingPool.getBrowser(id, opts)
             .catch((e) => {
                 this._launchNextBrowser();
                 return Promise.reject(e);
             });
     }
 
+    _lookAtNextRequest() {
+        return this._highPriorityRequestQueue.get(0) || this._requestQueue.get(0);
+    }
+
     _launchNextBrowser() {
         const queued = this._highPriorityRequestQueue.shift() || this._requestQueue.shift();
+
         if (queued) {
-            this.log('has queued requests');
+            const compositeId = buildCompositeBrowserId(queued.id, queued.version);
+
+            this.log(`has queued requests for ${compositeId}`);
             this.log(`remaining queue length: ${this._requestQueue.length}`);
-            this._newBrowser(queued.id)
+            this._newBrowser(queued.id, {version: queued.version})
                 .then(queued.resolve, queued.reject);
         } else {
             this._launched--;

diff --git a/lib/browser-pool/limited-use-set.js b/lib/browser-pool/limited-use-set.js
@@ -1,5 +1,6 @@
 'use strict';
 
+const _ = require('lodash');
 const Promise = require('bluebird');
 const debug = require('debug');
 
@@ -19,19 +20,23 @@ module.exports = class LimitedUseSet {
         this._useCounts = new WeakMap();
         this._useLimit = opts.useLimit;
         this._finalize = opts.finalize;
+        this._formatItem = opts.formatItem || _.identity;
         this._objects = [];
 
         this.log = debug(`${opts.logNamespace}:pool:limited-use-set`);
     }
 
     push(value) {
-        this.log(`push ${value}`);
+        const formatedItem = this._formatItem(value);
+
+        this.log(`push ${formatedItem}`);
+
         if (this._isOverLimit(value)) {
-            this.log('over limit, finalizing');
+            this.log(`over limit, finalizing ${formatedItem}`);
             return this._finalize(value);
         }
 
-        this.log('under limit');
+        this.log(`under limit for ${formatedItem}`);
         this._objects.push(value);
 
         return Promise.resolve();
@@ -52,9 +57,10 @@ module.exports = class LimitedUseSet {
 
         const result = this._objects.pop();
         const useCount = this._useCounts.get(result) || 0;
+        const formatedItem = this._formatItem(result);
 
-        this.log(`popping ${result}`);
-        this.log(`previous use count ${useCount}`);
+        this.log(`popping ${formatedItem}`);
+        this.log(`previous use count ${formatedItem}:${useCount}`);
 
         this._useCounts.set(result, useCount + 1);
 

diff --git a/lib/browser-pool/per-browser-limited-pool.js b/lib/browser-pool/per-browser-limited-pool.js
@@ -22,12 +22,14 @@ module.exports = class PerBrowserLimitedPool extends Pool {
     }
 
     getBrowser(id, opts) {
-        this.log(`request ${id}`);
+        this.log(`request ${id} with opts: ${JSON.stringify(opts)}`);
+
         return this._browserPools[id].getBrowser(id, opts);
     }
 
     freeBrowser(browser, opts) {
-        this.log(`free ${browser.id}`);
+        this.log(`free ${browser.fullId}`);
+
         return this._browserPools[browser.id].freeBrowser(browser, opts);
     }
 

diff --git a/lib/browser-pool/utils.js b/lib/browser-pool/utils.js
@@ -0,0 +1,3 @@
+exports.buildCompositeBrowserId = (browserId, version) => version
+    ? `${browserId}.${version}`
+    : browserId;
diff --git a/test/lib/browser-pool/basic-pool.js b/test/lib/browser-pool/basic-pool.js
@@ -37,6 +37,14 @@ describe('browser-pool/basic-pool', () => {
             .then(() => assert.calledWith(browserManager.create, 'broId'));
     });
 
+    it('should create new browser with specified version when requested', () => {
+        const browserManager = stubBrowserManager_();
+        const pool = mkPool_({browserManager});
+
+        return pool.getBrowser('broId', {version: '1.0'})
+            .then(() => assert.calledWith(browserManager.create, 'broId', '1.0'));
+    });
+
     it('should launch a browser', () => {
         const browser = stubBrowser();
         const browserManager = stubBrowserManager_();
@@ -45,7 +53,7 @@ describe('browser-pool/basic-pool', () => {
 
         return pool.getBrowser()
             .then(() => {
-                assert.calledOnce(browserManager.start);
+                assert.calledOnceWith(browserManager.start);
                 assert.calledWith(browserManager.start, browser);
             });
     });