[js] Do not hold command handle in the firefox.Binary class

This allows a single firefox.Binary instance to configure and launch
multiple Firefox processes.

Fixes #1116

Author: jleyba

javascript/node/selenium-webdriver/CHANGES.md

@@ -1,5 +1,15 @@
 ## v2.48.0-dev
 
+* FIXED: a single `firefox.Binary` instance may be used to configure and
+    launch multiple FirefoxDriver sessions.
+
+      var binary = new firefox.Binary();
+      var options = new firefox.Options().setBinary(binary);
+      var builder = new Builder().setFirefoxOptions(options);
+
+      var driver1 = builder.build();
+      var driver2 = builder.build();
+
 * FIXED: zip files created for transfer to a remote WebDriver server are no
     longer compressed. If the zip contained a file that was already compressed,
     the server would return an "invalid code lengths set" error.

javascript/node/selenium-webdriver/firefox/binary.js

@@ -133,7 +133,8 @@ function installNoFocusLibs(profileDir) {
 
 
 /**
- * Manages a Firefox subprocess configured for use with WebDriver.
+ * Provides a mechanism to configure and launch Firefox in a subprocess for
+ * use with WebDriver.
  *
  * @param {string=} opt_exe Path to the Firefox binary to use. If not
  *     specified, will attempt to locate Firefox on the current system.
@@ -157,9 +158,6 @@ var Binary = function(opt_exe) {
   this.env_['MOZ_CRASHREPORTER_DISABLE'] = '1';
   this.env_['MOZ_NO_REMOTE'] = '1';
   this.env_['NO_EM_RESTART'] = '1';
-
-  /** @private {promise.Promise.<!exec.Command>} */
-  this.command_ = null;
 };
 util.inherits(Binary, Serializable);
 
@@ -181,11 +179,11 @@ Binary.prototype.addArguments = function(var_args) {
 
 
 /**
- * Launches Firefox and eturns a promise that will be fulfilled when the process
- * terminates.
+ * Launches Firefox and returns a promise that will be fulfilled when the
+ * process terminates.
  * @param {string} profile Path to the profile directory to use.
- * @return {!promise.Promise.<!exec.Result>} A promise for the process result.
- * @throws {Error} If this instance has already been started.
+ * @return {!promise.Promise.<!exec.Command>} A promise for the handle to the
+ *     started subprocess.
  */
 Binary.prototype.launch = function(profile) {
   if (this.command_) {
@@ -200,7 +198,7 @@ Binary.prototype.launch = function(profile) {
 
   var args = ['-foreground'].concat(this.args_);
 
-  this.command_ = promise.when(this.exe_ || findFirefox(), function(firefox) {
+  return promise.when(this.exe_ || findFirefox(), function(firefox) {
     if (process.platform === 'win32' || process.platform === 'darwin') {
       return exec(firefox, {args: args, env: env});
     }
@@ -210,26 +208,6 @@ Binary.prototype.launch = function(profile) {
       return exec(firefox, {args: args, env: env});
     });
   });
-
-  return this.command_.then(function() {
-    // Don't return the actual command handle, just a promise to signal it has
-    // been started.
-  });
-};
-
-
-/**
- * Kills the managed Firefox process.
- * @return {!promise.Promise} A promise for when the process has terminated.
- */
-Binary.prototype.kill = function() {
-  if (!this.command_) {
-    return promise.defer();  // Not running.
-  }
-  return this.command_.then(function(command) {
-    command.kill();
-    return command.result();
-  });
 };
 
 

javascript/node/selenium-webdriver/firefox/index.js

@@ -239,39 +239,40 @@ var Driver = function(opt_config, opt_flow) {
   /** @private {?string} */
   this.profilePath_ = null;
 
-  /** @private {!Binary} */
-  this.binary_ = binary;
-
   var self = this;
-  var serverUrl = portprober.findFreePort().then(function(port) {
-    var prepareProfile;
+  var freePort = portprober.findFreePort();
+
+  /** @private */
+  this.command_ = freePort.then(function(port) {
     if (typeof profile === 'string') {
-      prepareProfile = decodeProfile(profile).then(function(dir) {
+      return decodeProfile(profile).then(function(dir) {
         var profile = new Profile(dir);
         profile.setPreference('webdriver_firefox_port', port);
         return profile.writeToDisk();
       });
     } else {
       profile.setPreference('webdriver_firefox_port', port);
-      prepareProfile = profile.writeToDisk();
+      return profile.writeToDisk();
     }
+  }).then(function(profileDir) {
+    self.profilePath_ = profileDir;
+    return binary.launch(profileDir);
+  });
 
-    return prepareProfile.then(function(dir) {
-      self.profilePath_ = dir;
-      return binary.launch(dir);
-    }).then(function() {
-      var serverUrl = url.format({
-        protocol: 'http',
-        hostname: net.getLoopbackAddress(),
-        port: port,
-        pathname: '/hub'
-      });
+  var serverUrl = this.command_
+      .then(function() { return freePort; })
+      .then(function(port) {
+        var serverUrl = url.format({
+          protocol: 'http',
+          hostname: net.getLoopbackAddress(),
+          port: port,
+          pathname: '/hub'
+        });
 
-      return httpUtil.waitForServer(serverUrl, 45 * 1000).then(function() {
-        return serverUrl;
+        return httpUtil.waitForServer(serverUrl, 45 * 1000).then(function() {
+          return serverUrl;
+        });
       });
-    });
-  });
 
   var executor = executors.createExecutor(serverUrl);
   var driver = webdriver.WebDriver.createSession(executor, caps, opt_flow);
@@ -296,7 +297,10 @@ Driver.prototype.quit = function() {
     var self = this;
     return Driver.super_.prototype.quit.call(this)
         .thenFinally(function() {
-          return self.binary_.kill();
+          return self.command_.then(function(command) {
+            command.kill();
+            return command.result();
+          });
         })
         .thenFinally(function() {
           if (self.profilePath_) {

javascript/node/selenium-webdriver/test/firefox/firefox_test.js

@@ -155,5 +155,29 @@ test.suite(function(env) {
         });
       });
     });
+
+    describe('binary management', function() {
+      var driver1, driver2;
+
+      test.ignore(env.isRemote).
+      it('can start multiple sessions with single binary instance', function() {
+        var options = new firefox.Options().setBinary(new firefox.Binary);
+        env.builder().setFirefoxOptions(options);
+        driver1 = env.builder().build();
+        driver2 = env.builder().build();
+        // Ok if this doesn't fail.
+      });
+
+      test.afterEach(function() {
+        if (driver1) {
+          driver1.quit();
+        }
+
+        if (driver2) {
+          driver2.quit();
+        }
+      });
+    });
+
   });
 }, {browsers: ['firefox']});