[JS] kill chromium service on quit (#10796)

* [JS] kill chromium service on quit * create new default service for every driver instance in chrome * modify getDefaultService to return new service in edge and chrome driver Co-authored-by: Diego Molina <[email protected]>
SeleniumHQ · Jul 4, 2022 · 632849c · MarkusJLechner · Jul 11, 2022 · titusfortner
1 parent ae46fd8
commit 632849c
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 66 deletions.
diff --git a/javascript/node/selenium-webdriver/chrome.js b/javascript/node/selenium-webdriver/chrome.js
@@ -140,7 +140,6 @@ const CHROMEDRIVER_EXE =
   process.platform === 'win32' ? 'chromedriver.exe' : 'chromedriver'
 
 /** @type {remote.DriverService} */
-let defaultService = null
 
 /**
  * Creates {@link selenium-webdriver/remote.DriverService} instances that manage
@@ -240,6 +239,14 @@ class Driver extends chromium.Driver {
       super.createSession(caps, opt_serviceExecutor)
     )
   }
+
+  /**
+   * returns new instance chrome driver service
+   * @returns {remote.DriverService}
+   */
+  static getDefaultService() {
+    return new ServiceBuilder().build()
+  }
 }
 
 /**
@@ -252,46 +259,14 @@ function locateSynchronously() {
   return io.findInPath(CHROMEDRIVER_EXE, true)
 }
 
-/**
- * Sets the default service to use for new ChromeDriver instances.
- * @param {!remote.DriverService} service The service to use.
- * @throws {Error} If the default service is currently running.
- */
-function setDefaultService(service) {
-  if (defaultService && defaultService.isRunning()) {
-    throw Error(
-      `The previously configured ChromeDriver service is still running. ` +
-        `You must shut it down before you may adjust its configuration.`
-    )
-  }
-  defaultService = service
-}
-
-/**
- * Returns the default ChromeDriver service. If such a service has not been
- * configured, one will be constructed using the default configuration for
- * a ChromeDriver executable found on the system PATH.
- * @return {!remote.DriverService} The default ChromeDriver service.
- */
-function getDefaultService() {
-  if (!defaultService) {
-    defaultService = new ServiceBuilder().build()
-  }
-  return defaultService
-}
-
 Options.prototype.CAPABILITY_KEY = 'goog:chromeOptions'
 Options.prototype.BROWSER_NAME_VALUE = Browser.CHROME
-Driver.getDefaultService = getDefaultService
 Driver.prototype.VENDOR_COMMAND_PREFIX = 'goog'
 
 // PUBLIC API
-
 module.exports = {
   Driver: Driver,
   Options,
   ServiceBuilder,
-  getDefaultService,
-  setDefaultService,
   locateSynchronously,
 }
diff --git a/javascript/node/selenium-webdriver/chromium.js b/javascript/node/selenium-webdriver/chromium.js
@@ -661,11 +661,13 @@ class Driver extends webdriver.WebDriver {
    */
   static createSession(caps, opt_serviceExecutor) {
     let executor
+    let onQuit
     if (opt_serviceExecutor instanceof http.Executor) {
       executor = opt_serviceExecutor
       configureExecutor(executor, this.VENDOR_COMMAND_PREFIX)
     } else {
       let service = opt_serviceExecutor || this.getDefaultService()
+      onQuit = () => service.kill()
       executor = createExecutor(service.start(), this.VENDOR_COMMAND_PREFIX)
     }
 
@@ -679,7 +681,7 @@ class Driver extends webdriver.WebDriver {
       }
     }
 
-    return /** @type {!Driver} */ (super.createSession(executor, caps))
+    return /** @type {!Driver} */ (super.createSession(executor, caps, onQuit))
   }
 
   /**

diff --git a/javascript/node/selenium-webdriver/edge.js b/javascript/node/selenium-webdriver/edge.js
@@ -90,7 +90,6 @@ const EDGEDRIVER_CHROMIUM_EXE =
   process.platform === 'win32' ? 'msedgedriver.exe' : 'msedgedriver'
 
 /** @type {remote.DriverService} */
-let defaultService = null
 
 /**
  * Creates {@link selenium-webdriver/remote.DriverService} instances that manage
@@ -157,6 +156,14 @@ class Driver extends chromium.Driver {
     )
   }
 
+  /**
+   * returns new instance of edge driver service
+   * @returns {remote.DriverService}
+   */
+  static getDefaultService() {
+    return new ServiceBuilder().build()
+  }
+
   /**
    * This function is a no-op as file detectors are not supported by this
    * implementation.
@@ -165,34 +172,6 @@ class Driver extends chromium.Driver {
   setFileDetector() {}
 }
 
-/**
- * Sets the default service to use for new Edge instances.
- * @param {!remote.DriverService} service The service to use.
- * @throws {Error} If the default service is currently running.
- */
-function setDefaultService(service) {
-  if (defaultService && defaultService.isRunning()) {
-    throw Error(
-      'The previously configured EdgeDriver service is still running. ' +
-        'You must shut it down before you may adjust its configuration.'
-    )
-  }
-  defaultService = service
-}
-
-/**
- * Returns the default Microsoft Edge driver service. If such a service has
- * not been configured, one will be constructed using the default configuration
- * for a MicrosoftWebDriver executable found on the system PATH.
- * @return {!remote.DriverService} The default Microsoft Edge driver service.
- */
-function getDefaultService() {
-  if (!defaultService) {
-    defaultService = new ServiceBuilder().build()
-  }
-  return defaultService
-}
-
 /**
  * _Synchronously_ attempts to locate the chromedriver executable on the current
  * system.
@@ -206,15 +185,12 @@ function locateSynchronously() {
 Options.prototype.BROWSER_NAME_VALUE = Browser.EDGE
 Options.prototype.CAPABILITY_KEY = 'ms:edgeOptions'
 Driver.prototype.VENDOR_CAPABILITY_PREFIX = 'ms'
-Driver.getDefaultService = getDefaultService
 
 // PUBLIC API
 
 module.exports = {
   Driver,
   Options,
   ServiceBuilder,
-  getDefaultService,
-  setDefaultService,
   locateSynchronously,
 }